Repository navigation
Fix direct controller for NotebookInstance to resolve references - #9810
Conversation
Update direct controller for NotebookInstance to use the correct patterns: 1. Call common.NormalizeReferences in AdapterForObject to resolve references like NetworkRef and SubnetRef. 2. Convert the KRM Spec to its Proto representation once in AdapterForObject and store it as desired in the adapter. 3. Update E2E golden files for notebooksinstance-full to include the correctly resolved network and subnet references. Fixes GoogleCloudPlatform#9804
| } | ||
|
|
||
| paths, err := common.CompareProtoMessage(desiredPb, a.actual, common.BasicDiff) | ||
| paths, err := common.CompareProtoMessage(a.desired, a.actual, common.BasicDiff) |
There was a problem hiding this comment.
Let's create a method compareNotebooks, as specified in the skill, just to follow the patterns
There was a problem hiding this comment.
Create a shared method updateStatus and call from Create and Update, as is specified in the skill
| return a.updateStatus(ctx, updateOp, updated) | ||
| } | ||
| return a.updateStatus(ctx, updateOp, a.actual) |
There was a problem hiding this comment.
Can we combine these two calls to updateStatus?
| clonedDesired := proto.Clone(desired).(*notebookspb.Instance) | ||
|
|
||
| populateDefaults := func(obj *notebookspb.Instance) { | ||
| // Even if empty, it's a good pattern to define and populate GCP/server defaults here |
There was a problem hiding this comment.
We don't need this comment here, the populateDefaults function is not empty. The pattern is to have a function to populate GCP defaults, even if that function is empty.
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: justinsb The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
1 similar comment
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: justinsb The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ab4e608
2 participants
Update direct controller for NotebookInstance to use the correct patterns:
Fixes #9804