Skip to content
This repository has been archived by the owner on Dec 31, 2023. It is now read-only.

docs: add Admin API samples for account management methods #58

Merged
merged 8 commits into from May 14, 2021

Conversation

ikuleshov
Copy link
Contributor

@ikuleshov ikuleshov commented May 7, 2021

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:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> 🦕

@product-auto-label product-auto-label bot added api: analyticsadmin Issues related to the googleapis/python-analytics-admin API. samples Issues that are directly related to samples. labels May 7, 2021
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label May 7, 2021
@snippet-bot
Copy link

snippet-bot bot commented May 7, 2021

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.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

@ikuleshov ikuleshov marked this pull request as ready for review May 7, 2021 21:21
@ikuleshov ikuleshov requested a review from a team as a code owner May 7, 2021 21:21
@ikuleshov ikuleshov requested a review from tmatsuo May 7, 2021 21:21
@ikuleshov
Copy link
Contributor Author

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.

@tmatsuo
Copy link

tmatsuo commented May 13, 2021

There are 7 possible violations for not having product prefix.

The snippet-bot thinks the product prefix is wrong.
Can you register the product in the devrel service?

Copy link

@tmatsuo tmatsuo left a 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

@@ -0,0 +1,55 @@
#!/usr/bin/env python

# Copyright 2021 Google Inc. All Rights Reserved.
Copy link

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

Copy link

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

from google.analytics.admin import AnalyticsAdminServiceClient


def run_sample():
Copy link

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?

Copy link
Contributor Author

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.

@leahecole
Copy link

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 noxfile_config_test.py file

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

@ikuleshov ikuleshov added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 14, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 14, 2021
@ikuleshov
Copy link
Contributor Author

Added type hints, with enforce_type_hints=True.

@ikuleshov
Copy link
Contributor Author

The proper region tag prefix seems to be set up, but the test still insists it is not.

@tmatsuo
Copy link

tmatsuo commented May 14, 2021

@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.

@ikuleshov ikuleshov merged commit 2ecc350 into googleapis:master May 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api: analyticsadmin Issues related to the googleapis/python-analytics-admin API. cla: yes This human has signed the Contributor License Agreement. samples Issues that are directly related to samples.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants