Skip to content
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

Custom Transport Configuration support #373

Conversation

dlabey
Copy link
Contributor

@dlabey dlabey commented Nov 2, 2023

  • Adds the concept of TransportConfiguration for
  • Adds support for setting a custom HTTP status code in the performCall method or use the same default bad request

Fixes #372

@armando-rodriguez-cko
Copy link
Contributor

Please @dlabey review all the tests, some test are failing

@dlabey
Copy link
Contributor Author

dlabey commented Nov 3, 2023

@armando-rodriguez-cko because I am on a fork I am not inheriting the IT secrets from the GitHub environment, and the tests have hardcoded processing channels... how do you advise I proceed?

@dlabey
Copy link
Contributor Author

dlabey commented Nov 3, 2023

@armando-rodriguez-cko I changed the build-pull-request file to use pull_request_target so it inherits the context of the source repo. According to https://blog.gitguardian.com/github-actions-security-cheat-sheet/, this can be insecure unless you manually approve the execution of workflows, which you are doing, so maybe this will help.

@armando-rodriguez-cko
Copy link
Contributor

Hi @dlabey, thank you for the changes but we can't admit the last commit, as you say this change is insecure. We will pass the tests with your previous changes and tell you how to proceed. Thank you.

@dlabey
Copy link
Contributor Author

dlabey commented Nov 6, 2023

@armando-rodriguez-cko I have merged it back, could you checkout this branch and run it with the secrets locally? Or could you take this branch and put it into the checkout owned repo for a PR to inherit the secrets?

@dlabey
Copy link
Contributor Author

dlabey commented Nov 8, 2023

@armando-rodriguez-cko any update? I cannot run the IT properly because my fork does not inherit the IT Checkout API keys which are coupled to the processing channels used in the IT tests...

@armando-rodriguez-cko
Copy link
Contributor

Hi @dlabey, we are working on it, sorry by the delay, we will try to finish it today.

@armando-rodriguez-cko armando-rodriguez-cko merged commit 8f6ea46 into checkout:master Nov 10, 2023
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Add ability to configure default response status code for ApacheHttpClientTransaport in performCall
3 participants