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

Discovery API should either work with version=None or raise an exception #971

Closed
DFrenkel opened this issue Jul 17, 2020 · 0 comments · Fixed by #975
Closed

Discovery API should either work with version=None or raise an exception #971

DFrenkel opened this issue Jul 17, 2020 · 0 comments · Fixed by #975
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@DFrenkel
Copy link
Contributor

DFrenkel commented Jul 17, 2020

Background

Google API Discovery V2 supports not supplying the version: for example, this URI happily returns what looks like the latest version of the service discovery document.

Requesting specific service version in discovery is probably a more common case (correct me if I am wrong here), but it comes with some maintenance cost: when a version of the service gets deprecated, customers have to explicitly move to another version when making discovery.build() calls.

Passing version=None to discovery.build(), having the intent to always get the latest version of the discovery document, results in a Bad Request errors because by default we prefer discovery v1, which does not support "default" or "latest" version semantics:

>>> from googleapiclient.discovery import build
>>> build("cloudresourcemanager", version=None)
Traceback (most recent call last):
...
googleapiclient.errors.HttpError: 
<HttpError 400 when requesting https://www.googleapis.com/discovery/v1/apis/cloudresourcemanager//rest
returned "Request contains an invalid argument.">

However, this works just fine:

>>> from googleapiclient.discovery import V2_DISCOVERY_URI
>>> build("cloudresourcemanager", version=None, discoveryServiceUrl=V2_DISCOVERY_URI)
...<googleapiclient.discovery.Resource object at 0x109986160>

This means that while we could fail over to the V2 discovery, we don't. We should probably correct this behavior in one of the two ways I am proposing below.

Proposal

Choice 1:

  • When version is not None, keep the order of discovery as is (i.e. V1 first, then V2 if V1 returns 404)
  • When version is None, drop V1 discovery (we know it will return an error) and only use V2, with logging a warning, something like Not providing specific service version during discovery may result in unexpected failures due to backwards compatibility

Choice 2:

  • Just raise an exception if we believe version is required during discovery and None is passed in.
@yoshi-automation yoshi-automation added the triage me I really want to be triaged. label Jul 18, 2020
@busunkim96 busunkim96 added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. and removed triage me I really want to be triaged. labels Jul 20, 2020
DFrenkel added a commit to DFrenkel/google-api-python-client that referenced this issue Jul 21, 2020
Passing version=None to discovery.build(), with intent to always
get the latest version of the discovery document,
no longer attempts to use Discovery V1, which fails with
Bad Request errors.
Instead, it uses Discovery V2, which supports null version use case.

Fixes: googleapis#971
DFrenkel added a commit to DFrenkel/google-api-python-client that referenced this issue Jul 23, 2020
Passing version=None to discovery.build(), with intent to always
get the latest version of the discovery document,
no longer attempts to use Discovery V1, which fails with
Bad Request errors.
Instead, it uses Discovery V2, which supports null version use case.

Fixes: googleapis#971
gcf-merge-on-green bot pushed a commit that referenced this issue Jul 30, 2020
Passing version=None to discovery.build(), with intent to always
get the latest version of the discovery document,
no longer attempts to use Discovery V1, which fails with
Bad Request errors.
Instead, it uses Discovery V2, which supports null version use case.

Fixes: #971
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants