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.
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
[feature][python] Support aliasing of API keys #6469
[feature][python] Support aliasing of API keys #6469
Changes from 12 commits
2b80b8a
1b4250c
93a7223
7ae3dc6
5b8f65f
0637df2
116fa25
eb6557e
49a4c3d
185650a
c5710b7
899e1f4
12e4f9d
b2d82c1
ea2706d
e8cdc89
3d5f7b7
e770aad
5527702
2502a2c
a7a9183
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
It may not be obvious but some generators samples have "live" tests which run pre-written tests against compiled code (see jersey2-java8). The tests aren't overwritten on generation (as a rule within DefaultGenerator).
This is documented in the description at the top of the file:
If you update this api key, it would require changing those other "live tests".
In this case, I'd recommend creating a separate smaller spec or adding another auth type outside of api_key which allows you to test the aliasing. That way you wouldn't have the conflict in the test.
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.
Jim does this point apply to this cordoned off spec file which is used for python-experimental only?
Is there another tag where we could ad this security scheme on to an use it there?
Should we use a new api tag?
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.
@spacether I don't know why a spec for python-only would be used by Java. My comment above is about changing a spec which is shared elsewhere and would require changes to hand-written Java tests.
I don't really follow your other questions. My recommendation was to add to the spec to avoid breaking those Java tests, or to create a standalone spec which demonstrates the aliasing behavior. If it was me, I'd do a separate minimal spec which demonstrates the behavior in unit tests then add the new alias parts to any existing spec to verify samples generation.
An issue with modifying existing specs that target swagger petstore (as does the one modified here) is that swagger petstore defines the header as
api_key
and someone wouldn't be able to use this spec to generate a valid client against that live API any longer. If we instead have another spec which doesn't target a real system's design (search forpony
,fruits
, and minimal "ping" specs), we don't risk breaking existing examples.Eventually, I think we need to better organize and document the test specs so we know each are "fakes", which are intended to be exemplar full APIs, and which ones are only meant to evaluate full feature support.
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 will create a separate spec to test API key aliasing feature and generate new Python client with tests for it.
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.
So the
modules/openapi-generator/src/test/resources/3_0/python-experimental/petstore-with-fake-endpoints-models-for-testing-with-http-signature.yaml
spec is used in 2 locations to generate samples:So I agree with Jim's assessment that we need a separate spec for python-experimental for this case because I do not see a PR adding support for api key aliasing in java-jersey2.
If this spec were only used by python-experimental then we could just:
fake
tag apiHow about this fix?
modules/openapi-generator/src/test/resources/3_0/python-experimental/petstore-with-fake-endpoints-models-for-testing-with-http-signature.yaml
as a resource that java-petstore-jersey2-java8.sh uses, maybe in a java-jersey2-java8 folderfake
api, add new security schemes so they are only used in that one endpoint on the fake apiThat fix meets these needs:
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.
@jimschubert does this proposed fix meet your needs or do you still prefer the small feature only spec?