Skip to content
This repository was archived by the owner on Mar 31, 2026. It is now read-only.

tests: pre-scrub old HMAC keys before testing creation#467

Merged
tseaver merged 2 commits into
masterfrom
334-pre-scrub-hmac-keys
Jun 21, 2021
Merged

tests: pre-scrub old HMAC keys before testing creation#467
tseaver merged 2 commits into
masterfrom
334-pre-scrub-hmac-keys

Conversation

@tseaver
Copy link
Copy Markdown
Contributor

@tseaver tseaver commented Jun 14, 2021

Avoids hitting 5-key-per-service-account quota.

Closes #334.

@tseaver tseaver requested review from a team, andrewsg and frankyn June 14, 2021 16:23
@google-cla google-cla Bot added the cla: yes This human has signed the Contributor License Agreement. label Jun 14, 2021
@product-auto-label product-auto-label Bot added the api: storage Issues related to the googleapis/python-storage API. label Jun 14, 2021
@tseaver
Copy link
Copy Markdown
Contributor Author

tseaver commented Jun 14, 2021

Note to reviewers: I created a key manually and, inside the testcase, hacked its time_created to be older than a day, just to ensure that the pre-scrub worked as expected.

@tseaver tseaver requested a review from chrisrossi June 14, 2021 20:03
@tseaver tseaver added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 18, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 18, 2021
@tseaver tseaver force-pushed the 334-pre-scrub-hmac-keys branch from 7190682 to bdbc765 Compare June 21, 2021 15:59

before_keys = set(Config.CLIENT.list_hmac_keys())

now = datetime.datetime.utcnow().replace(tzinfo=_helpers.UTC)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

With pytz in the standard library now, which includes a utc, is there any need to still use our own handrolled?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

$ python3.6
Python 3.6.10 (default, Apr 20 2021, 13:17:17) 
[GCC 9.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import pytz
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ModuleNotFoundError: No module named 'pytz'
>>>

See googleapis/python-api-core@61490f5 (part of googleapis/python-api-core#212), which uses datetime.timezone.utc.

GCS is still stuck supporting Python 2.7, however.

Copy link
Copy Markdown

@chrisrossi chrisrossi left a comment

Choose a reason for hiding this comment

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

LGTM. This feels like a think that should be a fixture, to me, but if it really only happens once, I guess it can go either way.

tseaver added 2 commits June 21, 2021 12:22
Avoids hitting 5-key-per-service-account quota.

Closes #334.
Addresses review comment.
@tseaver tseaver force-pushed the 334-pre-scrub-hmac-keys branch from bdbc765 to c07995d Compare June 21, 2021 16:22
@tseaver
Copy link
Copy Markdown
Contributor Author

tseaver commented Jun 21, 2021

@chrisrossi c07995d moves the pre-scrub logic to a helper method (these systests aren't pytest-fixtures-ified), so that any future systest which might create HMAC keys can reuse it. Opened #475 to track the need to refactor systests.

@tseaver tseaver merged commit cf22a11 into master Jun 21, 2021
@tseaver tseaver deleted the 334-pre-scrub-hmac-keys branch June 21, 2021 16:59
cojenco pushed a commit to cojenco/python-storage that referenced this pull request Oct 13, 2021
cojenco pushed a commit to cojenco/python-storage that referenced this pull request Oct 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api: storage Issues related to the googleapis/python-storage API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

'test_hmac_key_crud` test flakes

3 participants