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

allow API calls without GAE context #284

Merged
merged 2 commits into from
Jul 18, 2022
Merged

allow API calls without GAE context #284

merged 2 commits into from
Jul 18, 2022

Conversation

zevdg
Copy link
Collaborator

@zevdg zevdg commented Jul 14, 2022

The ApiHost used to require a security ticket for all API calls, so the
client side code used to be able to assume that any request without one
was invalid and reject it. The backend now is able to handle requests
without security tickets in many cases, so the client side check is now
just getting in the way. This change removes that check and lets the
backend attempt to handle all requests.

The way the client implementation happened to require a security ticket
was actually by requiring a GAE context. This change removes that
constraint as well and removes the now-unecessary BackgroundContext.

Also includes some minor cleanups/refactoring

  • construct the apiURL on demand instead of at the beginning of every request
  • decouple aetest's apiURL override from gae's internal context
  • remove unused "release funcs" from aetest
  • switch golang.org/x/net/context to stdlib context

@zevdg zevdg force-pushed the noticket branch 5 times, most recently from d91a522 to 82c5a38 Compare July 14, 2022 22:11
The ApiHost used to require a security ticket for all API calls, so the
client side code used to be able to assume that  any request without one
was invalid and reject it. The backend now is able to handle requests
without security tickets in many cases, so the client side check is now
just getting in the way. This change removes that check and lets the
backend attempt to handle all requests.

The way the client implementation happened to require a security ticket
was actually by requiring a GAE context. This change removes that
constraint as well and removes the now-unecessary BackgroundContext.
@jinglundong
Copy link
Collaborator

This is exciting. I may need to take a look tomorrow though. I'm fixing a customer issue.

@zevdg zevdg merged commit 37ff2b8 into golang:master Jul 18, 2022
@zevdg zevdg deleted the noticket branch July 18, 2022 15:05
zevdg added a commit to zevdg/appengine that referenced this pull request Sep 8, 2022
This makes the test more hermetic by avoiding the need to set env vars, and it also avoids some unecessary duplication of test helper logic by leveraging some of aetest's underlying implementation.

This change was originally part of golang#284, but I split it out because it's not compatible with v1's log flushing tests, and it would have added unecessary noise to that PR.
zevdg added a commit to zevdg/appengine that referenced this pull request Sep 8, 2022
This makes the test more hermetic by avoiding the need to set env vars, and it also avoids some unecessary duplication of test helper logic by leveraging some of aetest's underlying implementation.

This change was originally part of golang#284, but I split it out because it's not compatible with v1's log flushing tests, and it would have added unecessary noise to that PR.
zevdg added a commit to zevdg/appengine that referenced this pull request Sep 8, 2022
This makes the test more hermetic by avoiding the need to set env vars, and it also avoids some unecessary duplication of test helper logic by leveraging some of aetest's underlying implementation.

This change was originally part of golang#284, but I split it out because it's not compatible with v1's log flushing tests, and it would have added unecessary noise to that PR.
zevdg added a commit to zevdg/appengine that referenced this pull request Sep 13, 2022
This makes the test more hermetic by avoiding the need to set env vars, and it also avoids some unecessary duplication of test helper logic by leveraging some of aetest's underlying implementation.

This change was originally part of golang#284, but I split it out because it's not compatible with v1's log flushing tests, and it would have added unecessary noise to that PR.
ludoch pushed a commit that referenced this pull request Mar 16, 2023
This makes the test more hermetic by avoiding the need to set env vars, and it also avoids some unecessary duplication of test helper logic by leveraging some of aetest's underlying implementation.

This change was originally part of #284, but I split it out because it's not compatible with v1's log flushing tests, and it would have added unecessary noise to that PR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants