-
Notifications
You must be signed in to change notification settings - Fork 97
Add AWS IAM authorization support #64
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
Conversation
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 have left a few comments specific to design and implementation. A more general comment is that none of the tests are updated. We need to update the test classes to verify the modified functionalities.
@mghoneimy thank you for a review. Fixed the comments, but some things are to be discussed, looking forward. |
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.
We are very close here. The comments I added are mostly small. I'm trying to figure out why travis builds stopped on build requests too. Once both these things are done, we should be good to go.
Tests are fixed now |
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.
Thank you for addressing all the previous comments and fixing the tests!
I think that one comment is was not completely addressed and I think that I found a problem worth looking into before we merge this in.
Can you rebase your branch on the latest updates on |
Thanks for addressing all the comments. I accepted the changes, but again, I still need need you to rebase your changes on master before we can merge the PR. |
Great! Thank you. |
Hi, does anyone have an example of how the code should look like with this? As in, when making a query to a GraphQL endpoint with IAM auth. |
No description provided.