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.

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

[editor][server endpoints][2/n]: Add params arg to run command and check for undefined prompt_name #613

Merged
merged 1 commit into from
Jan 2, 2024

Conversation

rossdanlm
Copy link
Contributor

@rossdanlm rossdanlm commented Dec 26, 2023

[editor][server endpoints][2/n]: Add params arg to run command and check for undefined prompt_name

TSIA, pretty simple otherwise we'd be using empty params field and that would error. For the future, I created issue #671 where we shouldn't even need to pass in params into run and this works implicitly

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/run -d '{"prompt_name":"get_activities"}' -X POST -H 'Content-Type: application/json'

Without params

Screen.Recording.2024-01-02.at.12.55.58.mov

With params

Screen.Recording.2024-01-02.at.12.56.59.mov

@rholinshead
Copy link
Contributor

LGTM - fwiw I don't know that this will be needed for the editor since the params would be saved into the aiconfig when changed in the UI

Comment on lines 122 to 130
prompt_name : str = request_json.get("prompt_name", None)
if prompt_name is None:
return HttpResponseWithAIConfig(
message="No prompt name provided, cannot execute `run` command",
code=400,
aiconfig=None,
).to_flask_format()
params : str = request_json.get("params", None)
stream : bool = request_json.get("stream", True)
Copy link
Contributor

Choose a reason for hiding this comment

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

nits:
prompt_name is a str | None
please use the linter and/or vscode for pyright + black formatting.

autofix just these files, one-liner:

fd --glob '*.py' python/src/aiconfig/editor/server | xargs python -m 'scripts.lint' --mode=fix --files

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Awesome thanks for the auto-fix command!

rossdanlm pushed a commit that referenced this pull request Dec 28, 2023
Not being used, let's get rid of it dawg!

I also ran the linter based on Jonathan's command from this comment: #613 (comment)

```
fd --glob '*.py'  python/src/aiconfig/editor/server | xargs python -m 'scripts.lint' --mode=fix --files
```

^ (btw you can install fd on Mac using `brew install fd`, looks pretty cool). It actually seems pretty cool https://github.com/sharkdp/fd?tab=readme-ov-file
rossdanlm added a commit that referenced this pull request Dec 28, 2023
Remove declaration of generic T from server.py


Not being used, let's get rid of it dawg!

I also ran the linter based on Jonathan's command from this comment:
#613 (comment)

```
fd --glob '*.py'  python/src/aiconfig/editor/server | xargs python -m 'scripts.lint' --mode=fix --files
```

^ (btw you can install fd on Mac using `brew install fd`, looks pretty
cool). It actually seems pretty cool
https://github.com/sharkdp/fd?tab=readme-ov-file
@rossdanlm rossdanlm changed the title [editor][server endpoints][2/n]: Add params arg to run command [editor][server endpoints][2/n]: Add params arg to run command and check for undefined prompt_name Dec 29, 2023
…eck for undefined prompt_name

TSIA, pretty simple otherwise we'd be using empty params field and that would error. For the future, I created issue #671 where we shouldn't even need to pass in params into run and this works implicitly

## 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/run -d '{"prompt_name":"get_activities"}' -X POST -H 'Content-Type: application/json'
```

Without params

https://github.com/lastmile-ai/aiconfig/assets/151060367/7233a4db-9042-4fa9-affb-0f854ae8a50e


With params

https://github.com/lastmile-ai/aiconfig/assets/151060367/6c078804-ea66-4358-824d-21d0fe26bafa
@rossdanlm rossdanlm merged commit 505a3ad into main Jan 2, 2024
2 checks passed
@rossdanlm rossdanlm deleted the pr613 branch January 2, 2024 18:00
Victor-Su-Ortiz pushed a commit to Victor-Su-Ortiz/aiconfig that referenced this pull request Jan 4, 2024
Not being used, let's get rid of it dawg!

I also ran the linter based on Jonathan's command from this comment: lastmile-ai#613 (comment)

```
fd --glob '*.py'  python/src/aiconfig/editor/server | xargs python -m 'scripts.lint' --mode=fix --files
```

^ (btw you can install fd on Mac using `brew install fd`, looks pretty cool). It actually seems pretty cool https://github.com/sharkdp/fd?tab=readme-ov-file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants