Create set_name endpoint #692
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 5 commits
January 1, 2024 21:59
**Disclaimer:** I know this is 400+ lines changed, but ~300 of those are just adding/refactoring automated tests!!! We already had `get_prompt_parameters`, but this isn't good enough if parameters aren't defined locally and we want to bubble up to the aiconfig defaults. See comment in #661 (comment). This is important so that we can ensure we have valid params if not defined in the correct section of our AIConfig schema. This is similar in theme to #600 The only callsite we have today for this is in python in the `params.py` function, so I updated that callsite too. I don't know why it doesn't exist in Typescript (cc @rholinshead) but I also added this functionality in typescript in #669 (I'm not as familiar with automated testing there so will ask Ryan for a code pointer to help get me started) ## Test plan Added a bunch of automated tests which you can run by doing going to the `aiconfig/python` dir and running `pytest`
… defined before
Before this function would have failed to set any parameters if:
1. The prompt_name was passed in but `prompt.metadata` (possible) or `prompt.metadata.parameters` (not possible because it defaults to `{}`) was `None`
2. The prompt_name was not passed in (so update aiconfig) but the `aiconfig.metadata.parameters` was `None` (this is actually never possible because we default to using `{}`, but still just good to be thorough and future-proof)
See example below
<img width="410" alt="Screenshot 2023-12-29 at 01 04 48" src="https://github.com/lastmile-ai/aiconfig/assets/151060367/d3db50c7-866c-4f05-a279-284baf97d595">
## Test plan
Added automated tests: `pytest -vv`
TSIA, backend implementation of `update_model()`. The other changes were from running the auto-formatter: ``` fd --glob '*.py' python/src/aiconfig/editor/server | xargs python -m 'scripts.lint' --mode=fix --files ``` ## Test Plan Follow dev README to setup the local editor: https://github.com/lastmile-ai/aiconfig/tree/main/python/src/aiconfig/editor#dev, then run this command ``` curl http://localhost:8080/api/update_model -d curl http://localhost:8080/api/update_model -d '{"prompt_name":"get_activities", "model_name": "gpt-4", "settings": {"top_p": 0.9}}' -X POST -H 'Content-Type: application/json' ``` Notice that the model name is updated to `gpt-4`, and the `top_p` is now at 0.9 https://github.com/lastmile-ai/aiconfig/assets/151060367/3031fa59-8925-495c-a5eb-e56ec65b7fba
TSIA, this relies on #670 being unblocked ## Test Plan Follow dev README to setup the local editor: https://github.com/lastmile-ai/aiconfig/tree/main/python/src/aiconfig/editor#dev, then run this command ``` curl http://localhost:8080/api/set_parameter -d '{"prompt_name":"get_activities", "parameter_name": "city", "parameter_value": "New York"}' -X POST -H 'Content-Type: application/json' ``` Notice that the parameters are now set. We already have automated testing from #670 so we know the functionality works as long as this is connected to the client (pretty great!) https://github.com/lastmile-ai/aiconfig/assets/151060367/1be05d7e-63fe-4d6d-9036-6f8d4d004134
TSIA ## Test Plan Follow dev README to setup the local editor: https://github.com/lastmile-ai/aiconfig/tree/main/python/src/aiconfig/editor#dev, then run this command ``` curl http://localhost:8080/api/delete_parameter -d '{"prompt_name":"get_activities", "parameter_name": "city"}' -X POST -H 'Content-Type: application/json' ``` Notice that the parameters are now deleted Prompt-level https://github.com/lastmile-ai/aiconfig/assets/151060367/866db51d-6b84-438d-ac4e-483d0cfbaa40 AIConfig-level https://github.com/lastmile-ai/aiconfig/assets/151060367/dcd47f62-875c-40ac-9e81-efcd8cf9b45a
rossdanlm
requested review from
saqadri,
rholinshead,
suyoglastmileai,
Ankush-lastmile and
jonathanlastmileai
as code owners
This was referenced Jan 2, 2024
TSIA ## Test Plan Follow dev README to setup the local editor: https://github.com/lastmile-ai/aiconfig/tree/main/python/src/aiconfig/editor#dev, then run this command ``` curl http://localhost:8080/api/set_name -d '{"name":"cool_new_name"}' -X POST -H 'Content-Type: application/json' ``` https://github.com/lastmile-ai/aiconfig/assets/151060367/efcb7ceb-e390-4208-93b3-3effd5d66f8b
Comment on lines
273
to
274
| operation_args: Result[OpArgs, str] = result.Ok(OpArgs({"parameter_name": parameter_name, "prompt_name": prompt_name})) | ||
| return run_aiconfig_operation_with_op_args(aiconfig, "delete_parameter", operation, operation_args) |
There was a problem hiding this comment.
Is this auto-linting? Can you try to do that atomically for large sets of files in their own PRs, or even better do it before submitting the PR that modifies that specific code?
There was a problem hiding this comment.
You're right, my bad I forgot to do it last time
Comment on lines
+283
to
+287
| name: str | None = request_json.get("name") | ||
|
|
||
| operation = make_op_run_method(MethodName("set_name")) | ||
| operation_args: Result[OpArgs, str] = result.Ok(OpArgs({"name": name})) | ||
| return run_aiconfig_operation_with_op_args(aiconfig, "set_name", operation, operation_args) |
There was a problem hiding this comment.
I'm fairly sure you can use the wrapper function here (see a previous comment). If not, at this point I would probably refactor a bit to make it usable. I usually use the rule of thumb that a set of 3 cases justifies an abstraction.
If we hadn't done that to define make_op_run_method() and run_aiconfig_operation_with_op_args() in the first place, we would have either a lot more code duplication than this or a lot less robust error handling. We can probably go a little further here.
There was a problem hiding this comment.
Yea you're right, I was just copy pasting stuff
rholinshead
approved these changes
Jan 2, 2024
rossdanlm
added a commit
that referenced
this pull request
Jan 2, 2024
[python] Created `get_parameters` API **Disclaimer:** I know this is 400+ lines changed, but ~300 of those are just adding/refactoring automated tests!!! We already had `get_prompt_parameters`, but this isn't good enough if parameters aren't defined locally and we want to bubble up to the aiconfig defaults. See comment in #661 (comment). This is important so that we can ensure we have valid params if not defined in the correct section of our AIConfig schema. This is similar in theme to #600 The only callsite we have today for this is in python in the `params.py` function, so I updated that callsite too. I don't know why it doesn't exist in Typescript (cc @rholinshead) but I also added this functionality in typescript in #669 (I'm not as familiar with automated testing there so will ask Ryan for a code pointer to help get me started) ## Test plan Added a bunch of automated tests which you can run by doing going to the `aiconfig/python` dir and running `pytest` --- Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/lastmile-ai/aiconfig/pull/668). * #693 * #692 * #691 * #690 * #688 * #670 * __->__ #668
rossdanlm
added a commit
that referenced
this pull request
Jan 2, 2024
Create update_model endpoint TSIA, backend implementation of `update_model()`. The other changes were from running the auto-formatter: ``` fd --glob '*.py' python/src/aiconfig/editor/server | xargs python -m 'scripts.lint' --mode=fix --files ``` ## Test Plan Follow dev README to setup the local editor: https://github.com/lastmile-ai/aiconfig/tree/main/python/src/aiconfig/editor#dev, then run this command ``` curl http://localhost:8080/api/update_model -d curl http://localhost:8080/api/update_model -d '{"prompt_name":"get_activities", "model_name": "gpt-4", "settings": {"top_p": 0.9}}' -X POST -H 'Content-Type: application/json' ``` Notice that the model name is updated to `gpt-4`, and the `top_p` is now at 0.9 https://github.com/lastmile-ai/aiconfig/assets/151060367/3031fa59-8925-495c-a5eb-e56ec65b7fba --- Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/lastmile-ai/aiconfig/pull/688). * #693 * #692 * #691 * #690 * __->__ #688 * #670 * #668
rossdanlm
added a commit
that referenced
this pull request
Jan 2, 2024
Create delete_parameter endpoint TSIA ## Test Plan Follow dev README to setup the local editor: https://github.com/lastmile-ai/aiconfig/tree/main/python/src/aiconfig/editor#dev, then run this command ``` curl http://localhost:8080/api/delete_parameter -d '{"prompt_name":"get_activities", "parameter_name": "city"}' -X POST -H 'Content-Type: application/json' ``` Notice that the parameters are now deleted Prompt-level https://github.com/lastmile-ai/aiconfig/assets/151060367/866db51d-6b84-438d-ac4e-483d0cfbaa40 AIConfig-level https://github.com/lastmile-ai/aiconfig/assets/151060367/dcd47f62-875c-40ac-9e81-efcd8cf9b45a --- Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/lastmile-ai/aiconfig/pull/691). * #693 * #692 * __->__ #691 * #690 * #688 * #670 * #668
rossdanlm
added a commit
that referenced
this pull request
Jan 2, 2024
Create set_description endpoint TSIA ## Test Plan Follow dev README to setup the local editor: https://github.com/lastmile-ai/aiconfig/tree/main/python/src/aiconfig/editor#dev, then run this command ``` curl http://localhost:8080/api/set_description -d '{"description":"yo this is a cool new description get rekt dont @ me bro"}' -X POST -H 'Content-Type: application/json' ``` https://github.com/lastmile-ai/aiconfig/assets/151060367/0db278b3-8273-4178-8010-380538dcd0ca --- Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/lastmile-ai/aiconfig/pull/693). * __->__ #693 * #692 * #691 * #690 * #688 * #670 * #668
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Create set_name endpoint
TSIA
Test Plan
Follow dev README to setup the local editor: https://github.com/lastmile-ai/aiconfig/tree/main/python/src/aiconfig/editor#dev, then run this command
Screen.Recording.2024-01-01.at.23.32.44.mov
Stack created with Sapling. Best reviewed with ReviewStack.
set_parameter()to work if parameters haven't been defined before #670get_parametersAPI #668