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
Collaborator

@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 馃

busunkim96 added 2 commits Oct 5, 2020
client.quotas() now properly handles quota resources
with ``whitelistedKeySpecs``.
client.quotas() now properly handles quota resources
with ``whitelistedKeySpecs``.
@busunkim96 busunkim96 requested a review from as a code owner Oct 5, 2020
@google-cla google-cla bot added the cla: yes label Oct 5, 2020
@busunkim96 busunkim96 requested review from tswast and crwilcox Oct 5, 2020
tseaver
tseaver approved these changes Oct 6, 2020
return {
key: int(value) for key, value in resp["quota"].items() if key != "kind"
}
quotas = resp["quota"]
Copy link
Contributor

@crwilcox crwilcox Oct 6, 2020

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
Collaborator Author

@busunkim96 busunkim96 Oct 6, 2020

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

@crwilcox crwilcox Oct 6, 2020

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
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants