fix: fix deploy to same namespace if organization_id incorrectly set #55
Conversation
This file contains hidden or 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
cyclimse
force-pushed
the
fix/cant-deploy-same-namespace
branch
from
76bf86b to
2cc0fb6
Compare
cyclimse
commented
Mar 13, 2023
Comment on lines
-30
to
-33
| class ServerlessTestProject: | ||
| """ | ||
| ServerlessTestProject encapsulates the logic of | ||
| setting up a clean integration test environment. |
There was a problem hiding this comment.
I'm sorry for changing this again but I'm way happier with this new design. Last time I kinda gave up and decided to put everything in a single file and hide away this code as much as possible.
Now, I have separated the create project fixture from the CLI wrappers. To me, it seems a lot cleaner and makes it easier to manage.
cyclimse
force-pushed
the
fix/cant-deploy-same-namespace
branch
from
6988495 to
195bcab
Compare
3 tasks
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.
Summary
What's changed?
We now completely ignore the
default_organization_idparameter.Why do we need this?
We load the config from both env vars, CLI parameters, and config file. By default, most clients will have a
default_organization_idin their config file. However, if they use credentials for another account that has no rights in the organization, it can cause various deployment issues.Why remove it completely? It's not used by the framework.
How have you tested it?
Deployed some examples with the changes.
Checklist
Details
The cleaner fix would probably have been to not send
organization_idwhen it's unused for filtering, ie disable it only when it causes problems. However, the SDK will always send the client org id if it's explicitly set to a falsy value (check is done withor). To make that work, we would have to remove it from the client config, save it to a local and reapply later which seems cumbersome.