Skip to content

feat(tpu): add tpu queued resources startup script sample #9604

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.

Sign up for GitHub

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

Jump to bottom
Merged
TetyanaYahodska merged 26 commits into from
Nov 26, 2024
Merged

feat(tpu): add tpu queued resources startup script sample #9604

TetyanaYahodska merged 26 commits into from
Nov 26, 2024

Conversation

@TetyanaYahodska
Copy link
Contributor

@TetyanaYahodska TetyanaYahodska commented Oct 28, 2024 •
edited
Loading

Description

Python sample -> GoogleCloudPlatform/python-docs-samples#12716
Fixes #

Note: Before submitting a pull request, please open an issue for discussion if you are not associated with Google.

Checklist

  • I have followed Sample Format Guide
  • pom.xml parent set to latest shared-configuration
  • Appropriate changes to README are included in PR
  • These samples need a new API enabled in testing projects to pass (let us know which ones)
  • These samples need a new/updated env vars in testing projects set to pass (let us know which ones)
  • Tests pass: mvn clean verify required
  • Lint passes: mvn -P lint checkstyle:check required
  • Static Analysis: mvn -P lint clean compile pmd:cpd-check spotbugs:check advisory only
  • This sample adds a new sample directory, and I updated the CODEOWNERS file with the codeowners for this sample
  • This sample adds a new Product API, and I updated the Blunderbuss issue/PR auto-assigner with the codeowners for this sample
  • Please merge this PR for me once it is approved

@product-auto-label product-auto-label bot added the samples Issues that are directly related to samples. label Oct 28, 2024
@TetyanaYahodska TetyanaYahodska changed the title Tpu ququed resources startup script feat(tpu): add tpu queued resources startup script sample Oct 28, 2024
@TetyanaYahodska TetyanaYahodska added the kokoro:run Add this label to force Kokoro to re-run the tests. label Oct 28, 2024
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Oct 28, 2024
@TetyanaYahodska TetyanaYahodska added the kokoro:run Add this label to force Kokoro to re-run the tests. label Oct 28, 2024
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Oct 28, 2024
@TetyanaYahodska TetyanaYahodska added the kokoro:run Add this label to force Kokoro to re-run the tests. label Oct 28, 2024
@TetyanaYahodska TetyanaYahodska marked this pull request as ready for review October 28, 2024 14:31
@snippet-bot
Copy link

snippet-bot bot commented Oct 28, 2024 •
edited
Loading

Here is the summary of changes.

You are about to add 1 region tag.

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

@product-auto-label product-auto-label bot added the api: tpu Issues related to the Cloud TPU API. label Oct 29, 2024
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Oct 29, 2024
@Sita04 Sita04 assigned minherz and unassigned bourgeoisor Oct 30, 2024
minherz
minherz requested changes Oct 30, 2024
View reviewed changes
@TetyanaYahodska TetyanaYahodska requested a review from a team as a code owner October 30, 2024 21:08
@TetyanaYahodska TetyanaYahodska 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. labels Nov 18, 2024
@kokoro-team kokoro-team removed 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. labels Nov 18, 2024
@TetyanaYahodska TetyanaYahodska added the kokoro:run Add this label to force Kokoro to re-run the tests. label Nov 18, 2024
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Nov 18, 2024
@TetyanaYahodska TetyanaYahodska added the kokoro:run Add this label to force Kokoro to re-run the tests. label Nov 18, 2024
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Nov 18, 2024
@TetyanaYahodska TetyanaYahodska added the kokoro:run Add this label to force Kokoro to re-run the tests. label Nov 18, 2024
minherz
minherz requested changes Nov 19, 2024
View reviewed changes
Copy link
Contributor

@minherz minherz left a comment

Choose a reason for hiding this comment

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

please, fix the "delete" code sample. currently it cannot be effectively tested.
there are a couple of minor comments for your attention.

@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Nov 20, 2024
@TetyanaYahodska TetyanaYahodska added the kokoro:run Add this label to force Kokoro to re-run the tests. label Nov 20, 2024
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Nov 20, 2024
@TetyanaYahodska TetyanaYahodska added the kokoro:run Add this label to force Kokoro to re-run the tests. label Nov 20, 2024
minherz
minherz approved these changes Nov 25, 2024
View reviewed changes
Copy link
Contributor

@minherz minherz left a comment

Choose a reason for hiding this comment

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

lgtm. thank you for hard work.

tpu/src/test/java/tpu/QueuedResourceIT.java Outdated
Comment on lines 103 to 104
ByteArrayOutputStream bout = new ByteArrayOutputStream();
System.setOut(new PrintStream(bout));
Copy link
Contributor

@minherz minherz Nov 25, 2024

Choose a reason for hiding this comment

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

nit: why do you need this? it should be OK to print to stdout during the test unless you need to intercept this.

Copy link
Contributor Author

@TetyanaYahodska TetyanaYahodska Nov 26, 2024

Choose a reason for hiding this comment

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

This is an additional check that the code is working correctly.

Copy link
Contributor

@minherz minherz Nov 26, 2024

Choose a reason for hiding this comment

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

The validation that you reference is not exact. Additionally, it is based on the non-functional status. Such validations result in transient errors.
I am sure we discussed the guidelines that require not using stdout in test assertions in the past.
I would appreciate it if you will be more attentive to the guidelines and there will be no need to repeat the same review feedback in multiple submissions.

Copy link
Contributor Author

@TetyanaYahodska TetyanaYahodska Nov 26, 2024

Choose a reason for hiding this comment

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

Thank you! Deleted verification

@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Nov 26, 2024
@TetyanaYahodska TetyanaYahodska added the kokoro:run Add this label to force Kokoro to re-run the tests. label Nov 26, 2024
@TetyanaYahodska TetyanaYahodska merged commit 08aee80 into main Nov 26, 2024
@TetyanaYahodska TetyanaYahodska deleted the tpu_ququed_resources_startup_script branch November 26, 2024 17:00
rohithrajan-ai pushed a commit to rohithrajan-ai/java-docs-samples that referenced this pull request Nov 26, 2024
@TetyanaYahodska @rohithrajan-ai
feat(tpu): add tpu queued resources startup script sample (GoogleClou…
5d74977 
…dPlatform#9604)

* Added tpu_queued_resources_network sample

* Fixed samples and tests

* Fixed tests

* Changed CODEOWNERS

* Split samples, fixed startup script path

* Fixed style

* Added tag

* Added header

* Implemented tpu_queued_resources_startup_script sample, created test

* Fixed test, deleted cleanup method

* Fixed test, deleted cleanup method

* Fixed test

* Fixed naming

* Changed zone

* Fixed tests

* Fixed tests

* Increased timeout

* Fixed code as requested in comments

* Deleted settings

* Fixed test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@minherz minherz minherz approved these changes

@yoshi-approver yoshi-approver Awaiting requested review from yoshi-approver yoshi-approver is a code owner

Assignees

@minherz minherz

Labels

api: tpu Issues related to the Cloud TPU API. kokoro:run Add this label to force Kokoro to re-run the tests. samples Issues that are directly related to samples.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@TetyanaYahodska @minherz @bourgeoisor @kokoro-team