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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix client.quotas() method #24

Merged
merged 6 commits into from Oct 9, 2020
Merged

fix: fix client.quotas() method #24

merged 6 commits into from Oct 9, 2020

Conversation

busunkim96
Copy link
Contributor

@busunkim96 busunkim96 commented Oct 5, 2020

client.quotas() now properly handles quota resource with whitelistedKeySpecs.

This also removes the str -> int conversion for values in quotas. The API returns integers so there is no need to do the conversion. See https://cloud.google.com/dns/docs/reference/v1/projects#resource

Fixes #21 馃

client.quotas() now properly handles quota resources
with ``whitelistedKeySpecs``.
client.quotas() now properly handles quota resources
with ``whitelistedKeySpecs``.
@busunkim96 busunkim96 requested a review from a team as a code owner October 5, 2020 19:51
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Oct 5, 2020
@busunkim96 busunkim96 added the kokoro:run Add this label to force Kokoro to re-run the tests. label Oct 5, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Oct 5, 2020
return {
key: int(value) for key, value in resp["quota"].items() if key != "kind"
}
quotas = resp["quota"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there an advantage to getting the quotas and popping kind over a list comprehension? It appears the issue was the type conversion to int?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method started failing because there's a new key whitelistedKeySpecs in quota. str -> int conversion failed since it's a list

https://cloud.google.com/dns/docs/reference/v1/projects#resource

  "quota": {
    "kind": "dns#quota",
    "managedZones": integer,
    "rrsetsPerManagedZone": integer,
    "rrsetAdditionsPerChange": integer,
    "rrsetDeletionsPerChange": integer,
    "totalRrdataSizePerChange": integer,
    "resourceRecordsPerRrset": integer,
    "dnsKeysPerManagedZone": integer,
    "whitelistedKeySpecs": [
      {
        "kind": "dns#dnsKeySpec",
        "keyType": string,
        "algorithm": string,
        "keyLength": unsigned integer
      }
    ],
    "networksPerManagedZone": integer,
    "managedZonesPerNetwork": integer,
    "policies": integer,
    "networksPerPolicy": integer,
    "targetNameServersPerPolicy": integer,
    "targetNameServersPerManagedZone": integer
  }

Each entrywhitelistedKeySpecs also has a 'kind' that needs to be stripped. I found it easier to pop than to edit the existing list comprehension.

@@ -91,14 +91,23 @@ def quotas(self):

:rtype: mapping
:returns: keys for the mapping correspond to those of the ``quota``
sub-mapping of the project resource.
sub-mapping of the project resource. ``kind`` is stripped
from the results.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this to the comment. It seems we always stripped this away silently :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

client.quotas() error
4 participants