-
Notifications
You must be signed in to change notification settings - Fork 1
New process for running tests and publishing breadbox client #437
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
Conversation
Work in progress because all the steps around publishing have been removed
added httpx dependency added install of breadbox Bumping poetry version in case that helps more debugging build more debugging build more debugging build more debugging build more debugging more debugging more debugging more debugging build more debugging build more debugging build more debugging build more debugging build more debugging build added a new function for running a poetry command in virtual env added a new function for running a poetry command in virtual env added a new function for running a poetry command in virtual env added a new function for running a poetry command in virtual env added a new function for running a poetry command in virtual env added a new function for running a poetry command in virtual env Dropped unused settings variables for configuring broker url and backend
… regex and subprocess handling Co-authored-by: aider (vertex_ai/claude-3-7-sonnet@20250219) <[email protected]>
Co-authored-by: aider (vertex_ai/claude-3-7-sonnet@20250219) <[email protected]>
…nderstanding Co-authored-by: aider (vertex_ai/claude-3-7-sonnet@20250219) <[email protected]>
…ump script Co-authored-by: aider (vertex_ai/claude-3-7-sonnet@20250219) <[email protected]>
Co-authored-by: aider (vertex_ai/claude-3-7-sonnet@20250219) <[email protected]>
Co-authored-by: aider (vertex_ai/claude-3-7-sonnet@20250219) <[email protected]>
…y run mode Co-authored-by: aider (vertex_ai/claude-3-7-sonnet@20250219) <[email protected]>
Co-authored-by: aider (vertex_ai/claude-3-7-sonnet@20250219) <[email protected]>
Co-authored-by: aider (vertex_ai/claude-3-7-sonnet@20250219) <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! I've found it really time consuming in the past to wait for the build/publish in github to finish before I can use the updated/published client. I really like that we'll be able to have a bit more control over when/where we run this!
And I like that there's now a separate build/test workflow for the client too! 🎉
breadbox/breadbox/commands.py
Outdated
| def update_client(): | ||
| "Update the code for the breadbox client based. (Also saving out a new 'latest-breadbox-api.json' file)" | ||
|
|
||
| _export_api_spec("../breadbox-client/latest-breadbox-api.json") # TODO: call helper |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a TODO that still needs to be done?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure, but lets say "no" because no one is going to do this (or know what this refers to)
This is showing up as "new" because I moved commands.py from one directory to another and there were some changes so git doesn't seem to have figured out it was a rename and considers it as a delete+add.
I'll delete the TODO comment
breadbox/breadbox/commands.py
Outdated
| print("env") | ||
| print(os.environ) | ||
| print("cwd") | ||
| print(os.getcwd()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean to leave these print statements in here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope! I forgot about those. Left over from hacky debugging of what was going on when run by github.
Removing.
| # looking at the VIRTUAL_ENV enviornment variable. | ||
| # | ||
| # If you delete that variable, then `poetry` calls will correctly identify the venv based on the | ||
| # current working directory. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the comment explaining this! I've definitely run into a similar issue before and had been very confused by it!
|
@pgm is this ready to be merged in, or did you want to wait to talk about it at TPP? I am hoping to make some small breadbox client updates, and I don't want to create conflicts with your changes here - but my changes can wait if you wanted to talk about this before merging it in. |
This branch contains changes which strive to make the testing and publishing of the breadbox client more reliable.
Specifically, this consists of:
--dryrunfor debugging purposes.)