-
-
Couldn't load subscription status.
- Fork 14
Add python AWS Signature v4 support #64
Add python AWS Signature v4 support #64
Conversation
* Restructures endpoint data, modules written for responses * Endpoint docs improved, not done yet * Updates header_params * Updates path_parameters * Updates cookie_params, updates x_params to say key + input type * Samples updated * Fixes DefaultCodegenTests
* Java and template updates for separate request body module * Samples updated * Samples regenerated
* Adds parameter template and code to write it * Writes module for each parameter * Removes prependFormOrBodyParameters * Sample regenrated * Samples regenerated * Fixes readme example * simplifies setting items and additional_properties var names * Regenerates sample with fixed schema names * Samples regenerated * Fixes java docstirng typo * Fixes JavaModelTest tests * Fixes tests in JavaModelEnumTest * FIxes tests in JavaClientCodegenTest * FIxes tests in DefaultGeneratorTest * FIxes tests in DefaultCodegenTest * Adds HeaderParameterWithoutName to api_client * Sample regenerated, responses are now modules * Moves request body schemas to the root indentation level of the response modules * Adds code to generate response header modules * Updates fromParameter * Sample updated, fixed bug where form param was not seen as body * Samples regenerated * Fixes java tests
* python client modelPackage updated * Fixes links in model_doc template * Samples updated and python client path methods updated * Models sucessfully moved into components/schema in one sample * Replaces model import with components.schema * Fixes readme imports * Fixes model doc links from endpoint docs * Samples regenerated * Fixes manual test model imports * Samples regenerated, added init for test components * Removes model models folders * Removes lingering AbstractStep files, fixes its test * Removes lingering bad model test
move aws_sigv4 and http_signing modules into a signing submodule add sample with aws sigv4 signing Add pydocs for AWSSignatureV4Configuration module
Add manual tests for aws sigv4
...est/resources/3_0/python/petstore-with-fake-endpoints-models-for-testing-with-aws-sigv4.yaml
Show resolved
Hide resolved
Why are CI tests not running on this PR?
Update: CI was not set up to run on forked PRs. I fixed it.
Closing and re opening to kick off ci
fix import causing existing tests to fail
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.
How about renaming the spec file something like signing_spec.yaml?
How about outputting the sample into the features directory?
How about changing the package name to something like signing_spec_api?
With these changes I can reuse your sample spec to test the other signing module.
This sample is not related to petstore.
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.
How about also adding an init file to the signing folder?
I think that that is needed to get accurate coverage reports.
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 had added the template, but forget to add it as a supporting file! Good catch!
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 code should be route specific.
How about moving this code into api_client update_params_for_auth
And moving the invocation of that call after the used_headers.update?
https://github.com/openapi-json-schema-tools/openapi-json-schema-generator/blob/master/modules/openapi-json-schema-generator/src/main/resources/python/api_client.handlebars#L1063
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.
Sorry was taking a break all weekend so just getting back to this!
I like the idea of it being per route, however there is a comment above where used_headers.update is calling saying it should be called after any auth settings. Does that not apply anymore?
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.
No worries. Also I am on vacation so forgive some delayed responses from me here. It kind of doesn't apply. I wrote it. The default headers must come first. Update has to happen after, and those are spec defined headers. For your signing and the other security scheme they must come last because they depend upon the payload in general, right?
For the other security schemes, the Autherization header is disallowed as an explicit header input. So it is probably fine to update all security info last.
b63d721 to
d50d4a3
Compare
@jblakeney friendly ping. Do you still want this? I haven't seen any updates in a week.
@jblakeney friendly ping. Do you still want this? I haven't seen any updates in a week.
Hey, yes sorry I do plan to finish this. Its been a crazy work week. Ill aim to get the suggestions fixed up this week! Sorry for the delay!
No worries. I was just curious. Rebases on the 2_0_0 branch can be painful. If it is too much of a hassle, I find the easiest way to do it is to:
- squash the feature commits in your branch into 1 commit, copy it into a new branch
- locally delete your original branch
- make a new version of your original branch from the latest 2_0_0
- get cherry-pick your commit
- rebuild and regen samples
- force push your branch
No worries. I was just curious. Rebases on the 2_0_0 branch can be painful. If it is too much of a hassle, I find the easiest way to do it is to:
- squash the feature commits in your branch into 1 commit, copy it into a new branch
- locally delete your original branch
- make a new version of your original branch from the latest 2_0_0
- get cherry-pick your commit
- rebuild and regen samples
- force push your branch
Awesome ill give that a try, im usually pretty good at doing merge conflicts but we shall see! Ill give that an attempt tomorrow and ill let you know if I run into any issues.
4cb663e to
f5903a8
Compare
9c07ef5 to
529de3b
Compare
Sorry for the delay, just wanted to update you here. I got the conflcits resolved on a new branch, gonna make my changes and then ill get it merged into this branch and push it. Aiming to hopefully have that done by this weekend.
9d59527 to
8bbe6a5
Compare
8bbe6a5 to
3c2611d
Compare
Sorry again, im gonna close with for now. We do still need this but due to work deadlines before Christmas I havent had a chance to dedicate time to fix this up and properly test it. Ill re-open a PR once im ready to go. Sorry again!
No worries. When you have time you can add it later
Uh oh!
There was an error while loading. Please reload this page.
Work to implement feature in issue #56
PR checklist
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/python*.For Windows users, please run the script in Git BASH.