-
-
Notifications
You must be signed in to change notification settings - Fork 11
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
feat!: Handle 0.7 breaking changes and updates [SBK-369] #46
Conversation
faa28ba
to
40d54a0
Compare
8a12b07
to
1c944ed
Compare
1c944ed
to
c2f8cd9
Compare
b69d08e
to
bc5b6e1
Compare
@antazoey if you can check the updates I made to CLI |
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.
Couple questions inline. Generally looks good. Has this been tested? I'm especially curious if anything broke with the taskiq upgrade.
I ran the in-memory broker example from the README Would need more testing with some other usecases, specifically with the worker and with persistence |
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.
Tested locally. LGTM
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 think default=None
is no longer needed on the @network_option
. It doesn't work with ConnectedProviderCommand
anyway, as it will use the default network in that case, which is the same as not specifying default=None
.
NOTE: raises if mismatch
os.environ["SILVERBACK_NETWORK_CHOICE"] = val | ||
# NOTE: Make sure both of these have the same setting | ||
if env_network_choice := os.environ.get("SILVERBACK_NETWORK_CHOICE"): | ||
if val.network_choice != env_network_choice: |
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.
Note: @mikeshultz noticed if you use the shortened form in the environment variable, that the selection would be fully resolved and thus be mismatching
easy fix is to just update the environment variable to the fully resolved form
What I did
fixes: #
How I did it
How to verify it
Checklist