Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

👻 Config changes #526

Merged
merged 16 commits into from
Jan 13, 2025
Merged

👻 Config changes #526

merged 16 commits into from
Jan 13, 2025

Conversation

JonahSussman
Copy link
Contributor

@JonahSussman JonahSussman commented Dec 11, 2024 •
edited
Loading

This pr has the following changes:

  • config.toml is outta here
  • You supply initial log levels via command line arguments (see add_arguments in kai/rpc_server/main.py
  • All other config is supplied over an initialize request (see KaiRpcApplicationConfig in kai/rpc_server/server.py)

@JonahSussman JonahSussman force-pushed the config_changes branch 3 times, most recently from 7edf779 to 9b3f41d Compare December 12, 2024 17:01
sjd78 added a commit to sjd78/editor-extensions that referenced this pull request Dec 13, 2024
@sjd78
🐛 Add back default config.toml values
dc11abe 
Resolves: konveyor#160

Pending konveyor/kai#526, this change
is needed so the kai-rpc-server starts.  The initialize call
should still override any model configurations in the toml file.

Once the kai change to remove config.toml is merged, the
TODOs added by this change will need to be resolved.

Signed-off-by: Scott J Dickerson <[email protected]>
@sjd78 sjd78 mentioned this pull request Dec 13, 2024
Merged
djzager pushed a commit to konveyor/editor-extensions that referenced this pull request Dec 16, 2024
@sjd78 @rszwajko
🐛 Add back default config.toml values (#168)
1c7bd71 
Resolves: #160

Pending konveyor/kai#526, this change is needed
so the kai-rpc-server starts. The initialize call should still override
any model configurations in the toml file.

Once the kai change to remove `config.toml` is merged, the TODOs added
by this change will need to be resolved.

Signed-off-by: Scott J Dickerson <[email protected]>
Co-authored-by: Radoslaw Szwajkowski <[email protected]>
@sathiya06 sathiya06 mentioned this pull request Dec 27, 2024
Open
@JonahSussman
Moved old kai_config.py into kai_solution_server/
2ad07f2 
Signed-off-by: JonahSussman <[email protected]>
@JonahSussman
Miscellaneous spelling fixes
4ca6771 
Signed-off-by: JonahSussman <[email protected]>
@JonahSussman
Restored kai_config.py to kai/
7d3379e 
Signed-off-by: JonahSussman <[email protected]>
@JonahSussman
Changed constants.py to use pathlib.Path
b45ded2 
Signed-off-by: JonahSussman <[email protected]>
@JonahSussman
Further work
784e642 
Signed-off-by: JonahSussman <[email protected]>
@JonahSussman
Did further work
0daaf6d 
Signed-off-by: JonahSussman <[email protected]>
@JonahSussman
Removed circular import
319e769 
Signed-off-by: JonahSussman <[email protected]>
@JonahSussman
Did further work and re-enabled e2e gh action
5b2e22f 
Signed-off-by: JonahSussman <[email protected]>
@JonahSussman
Fixed debug driver
cb1a0c0 
Signed-off-by: JonahSussman <[email protected]>
@JonahSussman
Add back cli arg
cb64002 
Signed-off-by: JonahSussman <[email protected]>
@JonahSussman
Revert name change
f603b9d 
Signed-off-by: JonahSussman <[email protected]>
@JonahSussman JonahSussman mentioned this pull request Jan 10, 2025
Merged
@JonahSussman
Hack to fix cache
bb63cc8 
Signed-off-by: JonahSussman <[email protected]>
@JonahSussman
Another debugging thing
c600245 
Signed-off-by: JonahSussman <[email protected]>
@JonahSussman
Potential fix
6d980e8 
Signed-off-by: JonahSussman <[email protected]>
shawn-hurley
shawn-hurley approved these changes Jan 10, 2025
View reviewed changes
Copy link
Contributor

@shawn-hurley shawn-hurley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor points on logging otherwise LGTM

kai/llm_interfacing/model_provider.py Outdated Show resolved Hide resolved
kai/logging/logging.py Show resolved Hide resolved
@JonahSussman
Revert debugging changes
90de465 
Signed-off-by: JonahSussman <[email protected]>
@JonahSussman JonahSussman marked this pull request as ready for review January 13, 2025 18:28
@JonahSussman
Actually revert log
3b7114f 
Signed-off-by: JonahSussman <[email protected]>
@shawn-hurley shawn-hurley mentioned this pull request Jan 13, 2025
Closed
@sjd78
Copy link
Member

sjd78 commented Jan 13, 2025

It is safe to merge this before konveyor/editor-extensions#190 since we don't have scripted download of the workflow generated binaries quite yet. editor-extensions main will use released assets until we switch over. Hopefully this will be ready soon.

The ide devs can test 190 by manually downloading the binaries needed.

@shawn-hurley shawn-hurley merged commit 78f31fa into konveyor:main Jan 13, 2025
14 checks passed
sjd78 added a commit to sjd78/editor-extensions that referenced this pull request Jan 14, 2025
@sjd78
🌱 Update base configs and initialize call
def6506 
Based on changes to the kai runtime configurations
in konveyor/kai#526, update
the CLI arguments and initialize request object to
match.

Summary of changes:
  - Reorder the config field definitions

  - Use enum values in config definitions where available

  - Config keys with order 99 probably can be removed
    as they don't appear to be used

  - Aligned CLI arguments and initialize request payload
    types to the updated Kai configuration keys

  - Dropped config.toml handling since that file is no
    longer in use

Signed-off-by: Scott J Dickerson <[email protected]>
sjd78 added a commit to sjd78/editor-extensions that referenced this pull request Jan 15, 2025
@sjd78
🌱 Update config definitions, base configs and initialize call
5f9707b 
Based on changes to the kai runtime configurations
in konveyor/kai#526, update
the CLI arguments and initialize request object to
match.

Summary of changes:
  - Reorder the config field definitions

  - Use enum values in config definitions where available

  - Config keys with order 99 probably can be removed
    as they don't appear to be used

  - Aligned CLI arguments and initialize request payload
    types to the updated Kai configuration keys

  - Dropped config.toml handling since that file is no
    longer in use

Signed-off-by: Scott J Dickerson <[email protected]>
@sjd78 sjd78 mentioned this pull request Jan 15, 2025
Merged
sjd78 pushed a commit to konveyor/editor-extensions that referenced this pull request Jan 15, 2025
@JonahSussman
👻 Update analyzerClient.ts (#190)
461e9ce 
Update initialize parameters to align with
konveyor/kai#526. 

---------

Signed-off-by: Jonah Sussman <[email protected]>
sjd78 added a commit to sjd78/editor-extensions that referenced this pull request Jan 15, 2025
@sjd78
🌱 Update config definitions, base configs and initialize call
b43d52c 
Based on changes to the kai runtime configurations
in konveyor/kai#526, update
the CLI arguments and initialize request object to
match.

Summary of changes:
  - Reorder the config field definitions

  - Use enum values in config definitions where available

  - Config keys with order 99 probably can be removed
    as they don't appear to be used

  - Aligned CLI arguments and initialize request payload
    types to the updated Kai configuration keys

  - Dropped config.toml handling since that file is no
    longer in use

Signed-off-by: Scott J Dickerson <[email protected]>
sjd78 added a commit to konveyor/editor-extensions that referenced this pull request Jan 15, 2025
@sjd78
🌱 Update config definitions, base configs and initialize call (#197)
adefc8c 
Based on changes to the kai runtime configurations in
konveyor/kai#526, update the CLI arguments and
initialize request object to match.

Summary of changes:
  - Reorder the config field definitions

  - Use enum values in config definitions where available

  - Config keys related to analyzer configuration still remain but cannot
    be passed along to the analyzer, see [kai issue 550](konveyor/kai#550)

  - Aligned CLI arguments and initialize request payload types to the
    updated Kai configuration keys

  - Dropped config.toml handling since that file is no longer in use

Prerequisite for: #184

---------

Signed-off-by: Scott J Dickerson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shawn-hurley shawn-hurley shawn-hurley approved these changes

@pranavgaikwad pranavgaikwad pranavgaikwad approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@JonahSussman @sjd78 @shawn-hurley @pranavgaikwad