-
Notifications
You must be signed in to change notification settings - Fork 8
docs: add Admin API samples for account management methods #58
Conversation
Here is the summary of possible violations 😱 There are 7 possible violations for not having product prefix.
The end of the violation section. All the stuff below is FYI purposes only. Here is the summary of changes. You are about to add 7 region tags.
This comment is generated by snippet-bot.
|
Not sure why "Region tag product prefix" check is failing. We would like to use the "analyticsadmin" prefix for all samples (e.g. "analyticsadmin_accounts_get"), consistent with the Analytics Data API samples. |
The snippet-bot thinks the product prefix is wrong. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
noxfile.py seems old. Can you copy it from:
https://github.com/GoogleCloudPlatform/python-docs-samples/blob/master/noxfile-template.py
?
in noxifle_config.py, I think you want to change enforce_type_hints
to False.
@dandhlee
@leahecole Just in case I'm not saying something wrong.
The code seems good, but can you take care of:
- license holder names
- snippet-bot warning
- copy the latest noxfile.py
- turn off enforce_type_hints
Thanks
samples/account_summaries_list.py
Outdated
@@ -0,0 +1,55 @@ | |||
#!/usr/bin/env python | |||
|
|||
# Copyright 2021 Google Inc. All Rights Reserved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The copyright holder should be Google LLC
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And all the other files too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
samples/account_summaries_list.py
Outdated
from google.analytics.admin import AnalyticsAdminServiceClient | ||
|
||
|
||
def run_sample(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the purpose of this function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed occurrences of run_samples() where the code is trivial.
We typically do request that type annotations be enforced for new Python samples, so it would be preferred to keep "enforce type hints" on and add type hints to the functions. Regarding the latest noxfile for samples, copying the template totally works, but is there a reason why it's not picking up changes from the template in synthtool? You can also remove the
|
Added type hints, with enforce_type_hints=True. |
The proper region tag prefix seems to be set up, but the test still insists it is not. |
@ikuleshov The product is registered, but marked as private, so data file doesn't have the prefix yet. I'm ok if you ignore the failing status check. |
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> 🦕