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.

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

feat(compute): add compute instance attach regional disk force sample #9730

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open

feat(compute): add compute instance attach regional disk force sample #9730

wants to merge 17 commits into from
+156 −0

Conversation

TetyanaYahodska
Copy link
Contributor

@TetyanaYahodska TetyanaYahodska commented Nov 24, 2024 •
edited
Loading

Description

Documentation - https://cloud.google.com/compute/docs/disks/repd-failover?authuser=0#force-attach
Node.js sample - GoogleCloudPlatform/nodejs-docs-samples#3922
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 api: compute Issues related to the Compute Engine API. samples Issues that are directly related to samples. labels Nov 24, 2024
@TetyanaYahodska TetyanaYahodska added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 24, 2024
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 24, 2024
@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 24, 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 24, 2024
@TetyanaYahodska TetyanaYahodska added the kokoro:run Add this label to force Kokoro to re-run the tests. label Nov 24, 2024
@TetyanaYahodska TetyanaYahodska marked this pull request as ready for review November 24, 2024 19:51
@snippet-bot Snippet Bot
Copy link

snippet-bot bot commented Nov 24, 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

minherz
minherz requested changes Nov 27, 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, apply the feedback comments to the code sample file.

please, remove order in the e2e testing. i would strongly encourage to use mocking of the client library instead. it will simplify and speed up the testing.

compute/cloud-client/src/main/java/compute/disks/AttachRegionalDiskForce.java Outdated Show resolved Hide resolved
compute/cloud-client/src/main/java/compute/disks/AttachRegionalDiskForce.java Outdated Show resolved Hide resolved
compute/cloud-client/src/main/java/compute/disks/AttachRegionalDiskForce.java Outdated Show resolved Hide resolved
compute/cloud-client/src/main/java/compute/disks/AttachRegionalDiskForce.java Outdated Show resolved Hide resolved
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Nov 27, 2024
@TetyanaYahodska TetyanaYahodska added the kokoro:run Add this label to force Kokoro to re-run the tests. label Nov 27, 2024
@TetyanaYahodska
Copy link
Contributor Author

For the samples in the compute package we use real client, not mocked one. I followed this approach. Please leave it as is.

minherz
minherz reviewed Dec 9, 2024
View reviewed changes
Copy link
Contributor

@minherz minherz left a comment •
edited
Loading

Choose a reason for hiding this comment

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

please refactor tests to remove introduced sequential execution before proceed.
see a few minor comments for your consideration.

please, rebase the branch before moving forward since it conflicts with changes that were previously merged.

compute/cloud-client/src/main/java/compute/disks/AttachRegionalDiskForce.java Outdated
// Project ID or project number of the Cloud project you want to use.
String projectId = "YOUR_PROJECT_ID";
// Name of the zone of your compute instance.
String zone = "us-central1-a";
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: refactor the name to "instanceLocation" be more verbose

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you. Changed

compute/cloud-client/src/main/java/compute/disks/AttachRegionalDiskForce.java Outdated
// The name of the compute instance where you are adding the replicated disk.
String instanceName = "YOUR_INSTANCE_NAME";
// The region where your replicated disk is located.
String region = "us-central1";
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: refactor the name to "diskLocation" be more verbose

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you. Changed

compute/cloud-client/src/test/java/compute/disks/DisksIT.java Outdated
@@ -265,13 +275,15 @@ public void afterEach() {
}

@Test
@Order(1)
Copy link
Contributor

Choose a reason for hiding this comment

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

please consider to setup for each test separately instead of enforcing the order.
note that the order may fail upcoming tests despite that they are OK.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you. Deleted

@TetyanaYahodska TetyanaYahodska added the kokoro:run Add this label to force Kokoro to re-run the tests. label Dec 18, 2024
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Dec 18, 2024
@TetyanaYahodska TetyanaYahodska added the kokoro:run Add this label to force Kokoro to re-run the tests. label Dec 18, 2024
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Dec 18, 2024
@TetyanaYahodska TetyanaYahodska added the kokoro:run Add this label to force Kokoro to re-run the tests. label Dec 19, 2024
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Dec 19, 2024
@TetyanaYahodska TetyanaYahodska added the kokoro:run Add this label to force Kokoro to re-run the tests. label Dec 19, 2024
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Dec 19, 2024
@TetyanaYahodska TetyanaYahodska added the kokoro:run Add this label to force Kokoro to re-run the tests. label Dec 19, 2024
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Dec 19, 2024
@TetyanaYahodska TetyanaYahodska added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 19, 2024
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 19, 2024
@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 Dec 19, 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 Dec 20, 2024
@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 Dec 20, 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 Dec 20, 2024
@TetyanaYahodska TetyanaYahodska added the kokoro:run Add this label to force Kokoro to re-run the tests. label Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@minherz minherz minherz requested changes

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

Requested changes must be addressed to merge this pull request.

Assignees

@minherz minherz

Labels
api: compute Issues related to the Compute Engine 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 @Mukamik @kokoro-team