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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Fixed object paths in autogenerated code in owlbot.py #804

Merged
merged 4 commits into from Nov 1, 2023

Conversation

gkevinzheng
Copy link
Contributor

Fix object paths by doing a find/replace operation in owlbot.py, since the gapic objects are not included in the __init__.py of google.cloud.logging_v2.

Fixes #582

@gkevinzheng gkevinzheng requested review from a team as code owners October 26, 2023 19:20
@product-auto-label product-auto-label bot added the size: m Pull request size is medium. label Oct 26, 2023
@product-auto-label product-auto-label bot added the api: logging Issues related to the googleapis/python-logging API. label Oct 26, 2023
@gcf-owl-bot gcf-owl-bot bot requested a review from a team as a code owner October 26, 2023 19:22
@gcf-owl-bot gcf-owl-bot bot requested a review from nicain October 26, 2023 19:22
Copy link
Contributor

@daniel-sanche daniel-sanche left a comment

Choose a reason for hiding this comment

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

LGTM, with suggestions

# For autogenerated sample code, resolve object paths by finding the specific subpackage
# the object belongs to. This is because we leave out all autogenerated packages from the
# __init__.py of logging_v2. For now, this is manually copy-pasted from the __all__s of each
# subpackage's __init__.py.
Copy link
Contributor

Choose a reason for hiding this comment

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

we discussed the possibility of eventually restructuring the gapic/veneer files into different folders, which should remove the need for this fix, right?

It might be a good idea to open a bug for that and add a link to the comment here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Depending on how the gapic/veneer files are structured we might need to have a different fix for replacing logging_v2 with something like logging_v2.gapic, but yeah we wouldn't need a dictionary structure like it is now.


# Initialize request argument(s)
request = logging_v2.CopyLogEntriesRequest(
request = logging_v2.types.CopyLogEntriesRequest(
Copy link
Contributor

Choose a reason for hiding this comment

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

One thought: these generated samples aren't covered by tests. Have you considered adding tests for these to catch this kind of issue in the future, like the other snippets? Smoke tests could be helpful here (i.e. just executing each function and making sure nothing crashes)

There may be a simple way to programmatically load and execute each sample, but if it's bit more complicated we could open a bug for later

@gkevinzheng gkevinzheng added kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. and removed kokoro:run Add this label to force Kokoro to re-run the tests. labels Nov 1, 2023
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 1, 2023
@gkevinzheng gkevinzheng merged commit b14bb14 into main Nov 1, 2023
15 checks passed
@gkevinzheng gkevinzheng deleted the owlbot-samples-fix branch November 1, 2023 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: logging Issues related to the googleapis/python-logging API. size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix Autogenerated Samples
3 participants