feat: compute_consume_specific_shared_reservation

[gryczj]

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

[snippet-bot]

Here is the summary of changes.

You are about to add 1 region tag.

• compute/reservations/createInstanceToConsumeSharedReservation.js:20, tag compute_consume_specific_shared_reservation

---

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]

In this PR I also moved cleanup methods to after() to prevent keeping not used resources in our project after running the tests and to reduce the code.

[m-strzelczyk]

Can we rename to a more obvious names? Like reservationOwnerProjectID and reservationConsumerProjectID for example?

[gryczj]

@m-strzelczyk sure, fixed

[BigBlackWolf]

Hi @iennae, could you please take a look once again on this PR?

cc: @rsamborski

[iennae]

thanks @BigBlackWolf for the ping. Can you check the changes I made to the one file to clarify the comments? I think there is a lot of specific choices embedded in the samples that could use a little more detail and then some obvious comments that distract from the code. For example the instantiates aren't giving more context to the sample consumers. Could you make updates to the other files to match that style?

In general, I know there are other samples like this, but it really feels import with the number of choices that folks have with compute to better understand how to manage that complexity rather than hide it and end up with copied patterns over time with no understanding of the "why".

Thank you!

[iennae]

Aside from the comments needing improvement, I have some additional concerns. Refactoring to eliminate duplicate code and create helper functions would enhance readability and maintainability. These new helper functions could accept parameters like disk size, source image, and network settings, simplifying updates to this collection of samples and streamlining the samples across different attributes. Currently, the handcrafted samples require extensive reviews and introduce duplicate code.

I will seek a second opinion from the team tomorrow and update this PR accordingly. While I lean towards a conditional approval, I believe we can enhance consistency and style.

Thank you for your patience.

[BigBlackWolf]

I appreciate your review @iennae and thanks for adjusting comments, imho they became much more informative!
I agree, that it's worth to correct other samples, however it looks like out of the scope of this PR. As I see, the intention was to create compute_consume_specific_shared_reservation.
Don't you mind introducing your suggestions in a separate PR?

[iennae]

Heya, the problem is the other issues I raised and this PR has made me realize there is a lot of repetition in these set of samples whoch is going to make them harder to maintain. Do we have an assigned engineering team that will be ok maintaining these?

[BigBlackWolf]

Unfortunately, I am not aware of those.
Do you think changes, that help reduce repetition should be introduced in this PR?

[iennae]

I think refactoring the new sample that is getting added since it has the same duplicate functions could happen in this PR. Do you know if there is an engineering team that will take on maintenance? My primary concern is the folks who have to maintain these. To be clear, I'm nit saying all the existing samples need to get updated but the new one should be refactoring to make this duplication code maintainable.

[m-strzelczyk]

@iennae I agree that the Compute code samples suffer from a lot of code repetition, which makes the code samples harder to maintain and read.

However, helper functions are not that simple to implement with the way our code sample system works. We have two ways to work with this:

1. Have the documentation page pull multiple code samples, for example: helper function to make a disk, helper function to configure networking and main function that actually creates the new instance. This would require more maintenance on the side of the documentation and users wouldn't be able to simply copy-paste a working code sample from a single box.
2. Use a system similar to what I implemented for Compute in the Python repository, where helper functions and main samples get auto-merged into a final code sample file with single region tag. The downside of this is more complicated process of introducing new samples and the final code samples in Python tend to be longer than in other languages.

After years of using the Python way of "compiling" code samples, I'm not sure it was the best way of doing things. I hate code repetition, but on the other hand, I want our users, when faced with a single code sample, to have everything available to them in a very approachable way. It wouldn't be good if we, for example, used a helper function in a code samples and later forgot to include it in the documentation next to the code sample.

That said, I'll have a look at the code and comments to address your other suggestions.

[glasnt]

CLA check override note: previous executions worked https://github.com/GoogleCloudPlatform/nodejs-docs-samples/pull/3912/checks?check_run_id=34225819577