Skip to content

[iam] fix: use uuid4's first part for test#3565

Merged
tmatsuo merged 2 commits into
GoogleCloudPlatform:masterfrom
tmatsuo:fix-iam
Apr 28, 2020
Merged

[iam] fix: use uuid4's first part for test#3565
tmatsuo merged 2 commits into
GoogleCloudPlatform:masterfrom
tmatsuo:fix-iam

Conversation

@tmatsuo

@tmatsuo tmatsuo commented Apr 28, 2020

Copy link
Copy Markdown
Contributor

fixes #3499

@tmatsuo tmatsuo requested a review from a team as a code owner April 28, 2020 00:54
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 28, 2020
@tmatsuo tmatsuo requested a review from gguuss April 28, 2020 00:56
project_id = os.environ['GCLOUD_PROJECT']
rand = str(random.randint(0, 1000))
name = 'python-test-' + rand
name = 'python-test-{}'.format(str(uuid.uuid4()).split('-')[0])

@gguuss gguuss Apr 28, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I feel like a deterministic value based on machine ID / mac address would be ideal - are you just using UUID4 for a random value?

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.

Yes. Due to the length limitation, I needed to use only the first part, but it's still far better than random.randint(0, 1000).

@gguuss gguuss left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, random UUID is probably better than a random value to avoid machine collisions.

@tmatsuo tmatsuo merged commit 6e45bc9 into GoogleCloudPlatform:master Apr 28, 2020
@tmatsuo tmatsuo deleted the fix-iam branch April 28, 2020 01:50
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.

iam.api-client.service_accounts_test: test_service_accounts failed

3 participants