-
Notifications
You must be signed in to change notification settings - Fork 1
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
refactor(breadbox): remove & auto-generate breadbox client #119
base: master
Are you sure you want to change the base?
Conversation
This seems to be required for composite actions
Since defaults don't seem to be allowed in templates
breadbox-client/README.md
Outdated
In general, the `breadbox_client` directory is auto-generated in a github action. It is not checked in to the repo. | ||
|
||
If you would like to generate the client for local developmen/debugging, follow these steps: | ||
1. Install the `openapi-python-client` with the python environment manager of your choice. For example: `pipx install openapi-python-client==0.21.1` |
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.
I'm curious, why run this long command instead of ./bb update-client
like we've been doing to date?
If I'm making changes to breadbox and want to test the client in dev to evaluate them, I'd want to get a client using a newly generated api.json.
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.
This is one of the things I wanted to get input on, but after thinking about it a bit more, I think you're right that the best option is to keep using the ./bb update-client
command for local development.
Historically, I've had some issues using that command without doing all of the following steps (and I wanted to ask if others have had similar issues):
cd breadbox-client-generator/
poetry install && poetry shell
cd ../breadbox
./bb update-client
However, I suspect that might have actually been an issue with my local python/poetry configuration, because the ./bb update-client
command does seem to be working for me now (and I'm not sure what changed).
I am going to undo some of the changes I had made on this branch so we can keep using that command 👍
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.
FYI, I made the couple of changes we talked about. I also added a note to our check-in list so we remember to follow-up on potentially getting the ./bb update-client
command working in github actions. I'll wait to make any other changes to this branch until after we talk about that.
These changes include:
a.
pyright-breadbox-client
: validates that the breadbox-facade types are valid with the auto-generated clientb.
publish-breadbox-client
: publishes the breadbox-client module (including both the auto-generated code and the facade)I also had a couple of questions that I thought might be worth discussing. I added them to our breadbox meeting list here:
breadbox-client-generator
repo. I had an easier time setting up the github action without it (by using pip to just install the one package)I'll wait to merge these changes until after we talk through the open questions