feat(sidekick): create gated resource name heuristic foundation#4085
feat(sidekick): create gated resource name heuristic foundation#4085haphungw wants to merge 6 commits intogoogleapis:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request lays a solid foundation for the resource name heuristic, with clear and well-tested logic. The changes are well-structured and easy to follow. I have a couple of suggestions to enhance performance and improve comment clarity in the new code.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4085 +/- ##
==========================================
+ Coverage 81.95% 82.20% +0.25%
==========================================
Files 78 79 +1
Lines 6521 6526 +5
==========================================
+ Hits 5344 5365 +21
+ Misses 832 813 -19
- Partials 345 348 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
coryan
left a comment
There was a problem hiding this comment.
The file name is a blocker for me. The rest are questions or nits.
dbolduc
left a comment
There was a problem hiding this comment.
This approach could work, but my instinct is that we need to update the api.Model to hold candidate resources somewhere. (I think on a pathBinding). As the resources that might be targeted by a method do not depend on the language of the client library.
If we have some function like calculateResourceCandidates(...) for a path binding (or maybe method), then we would encode the heuristic in that logic. I think we would not have a HeuristicMetadata struct where the resource candidate logic is kinda split in two very different places.
Define HeuristicMetadata and include universtal GCP resource roots in BaseVocabulary.
bb386db to
6bb5051
Compare
We want our client libraries to include the resource name (like
projects/my-project/instances/my-vm) for tracing. This PR adds the foundational heuristic implementation to help sidekick figure out the names for those specific services.For #4018