Skip to content

Conversation

@gryczj
Copy link
Contributor

@gryczj gryczj commented Aug 26, 2024

Description

Fixes #

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

Checklist

  • I have followed guidelines from CONTRIBUTING.MD and Samples Style Guide
  • Tests pass: npm test (see Testing)
  • Lint pass: npm run lint (see Style)
  • 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)
  • This pull request is from a branch created directly off of GoogleCloudPlatform/nodejs-docs-samples. Not a fork.
  • This sample adds a new sample directory, and I updated the CODEOWNERS file with the codeowners for this sample
  • This sample adds a new sample directory, and I created GitHub Actions workflow 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

@gryczj gryczj self-assigned this Aug 26, 2024
@product-auto-label product-auto-label bot added samples Issues that are directly related to samples. api: compute Issues related to the Compute Engine API. labels Aug 26, 2024
@gryczj gryczj added kokoro:force-run Add this label to force Kokoro to re-run the tests. kokoro:run Add this label to force Kokoro to re-run the tests. labels Aug 26, 2024
@kokoro-team kokoro-team removed kokoro:force-run Add this label to force Kokoro to re-run the tests. kokoro:run Add this label to force Kokoro to re-run the tests. labels Aug 26, 2024
@gryczj gryczj force-pushed the compute_reservation_create branch from 3962724 to aa5f756 Compare August 26, 2024 09:03
@gryczj gryczj changed the title Compute reservation create feat: compute reservation create Aug 26, 2024
@gryczj gryczj force-pushed the compute_reservation_create branch 6 times, most recently from 6b92e7b to 9986010 Compare August 26, 2024 13:03
Copy link
Contributor

@subfuzion subfuzion left a comment

Choose a reason for hiding this comment

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

Hi @gryczj. Last week I helped with a few changes to try to stabilize ensuring resources didn't exist from previous failed tests and were removed after tests. I think you need to work with your team to come up with a more optimal solution for handling this cleanly and reliably. It seems that this is a problematic area (I had to insert delays with retries, but it felt like a bit of a brute force hack). Perhaps for compute instances, you really do need to separate these concerns into a more robust set up and tear down module with retries that you import that ensures this happens in a consistently reliable way before erroring. If it's this hard for us to do, then it's going to be hard for our customers as well, so let's convert this to draft mode while you take a bit of time to coordinate with your team to come up with something that showcases the proper way to handle this. Thank you!

@gryczj
Copy link
Contributor Author

gryczj commented Aug 27, 2024

Hi @gryczj. Last week I helped with a few changes to try to stabilize ensuring resources didn't exist from previous failed tests and were removed after tests. I think you need to work with your team to come up with a more optimal solution for handling this cleanly and reliably. It seems that this is a problematic area (I had to insert delays with retries, but it felt like a bit of a brute force hack). Perhaps for compute instances, you really do need to separate these concerns into a more robust set up and tear down module with retries that you import that ensures this happens in a consistently reliable way before erroring. If it's this hard for us to do, then it's going to be hard for our customers as well, so let's convert this to draft mode while you take a bit of time to coordinate with your team to come up with something that showcases the proper way to handle this. Thank you!

@subfuzion Please see: #3808

@gryczj gryczj force-pushed the compute_reservation_create branch 2 times, most recently from 99266b2 to 7aef226 Compare August 27, 2024 11:17
@gryczj gryczj force-pushed the compute_reservation_create branch 3 times, most recently from 23d1e3a to 1be31dd Compare August 30, 2024 14:33
@gryczj gryczj marked this pull request as ready for review August 30, 2024 14:43
@gryczj gryczj requested review from a team as code owners August 30, 2024 14:43
@snippet-bot
Copy link

snippet-bot bot commented Aug 30, 2024

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

@gryczj gryczj requested a review from subfuzion August 30, 2024 14:43
@gryczj

This comment was marked as resolved.

@subfuzion
Copy link
Contributor

@gryczj I merged #3808 and then updated this branch, but that doesn't seem to have resolved the issue, unfortunately:

https://github.com/GoogleCloudPlatform/nodejs-docs-samples/actions/runs/10686130157/job/29620561886?pr=3807#step:9:268

@subfuzion subfuzion assigned subfuzion and unassigned subfuzion Sep 3, 2024
@gryczj gryczj force-pushed the compute_reservation_create branch 6 times, most recently from d06ebfe to 9a50ec4 Compare September 4, 2024 09:58
@gryczj gryczj force-pushed the compute_reservation_create branch from 9a50ec4 to c8e5d6c Compare September 4, 2024 10:00
const projectId = await storagePoolClient.getProjectId();
// Name of the zone in which you want to create the storagePool.
const zone = 'europe-central2-b';
const zone = 'us-central1-a';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can't create storage pool in europe zone. Changed to US as it was before. The same problem was in Java directory: GoogleCloudPlatform/java-docs-samples@e243370

@gryczj gryczj requested a review from subfuzion September 4, 2024 12:04
@gryczj gryczj merged commit 2a47751 into main Sep 4, 2024
@gryczj gryczj deleted the compute_reservation_create branch September 4, 2024 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: compute Issues related to the Compute Engine API. samples Issues that are directly related to samples.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants