-
Notifications
You must be signed in to change notification settings - Fork 215
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.
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
enh: towards DataformRepository beta #2443
Conversation
Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
userPrivateKeySecretVersionRef: | ||
description: The name of the Secret Manager secret version | ||
to use as a ssh private key for Git operations. Must be | ||
in the format projects/*/secrets/*/versions/* . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: we should probably remove the "Must be in the format" here from the docstring, not a blocker for alpha though (and we might want a API linter in future!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As in we shouldn't share the expected format in the doc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should, but it goes on external
, not on the parent ref
return fmt.Errorf("converting GitRemoteSettings to api: %w", mapCtx.Err()) | ||
} | ||
|
||
if !reflect.DeepEqual(protoDesired, a.actual.GitRemoteSettings) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This bit (the reflect.DeepEqual) does look good though. We might also be able to use proto.Equal
Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
@@ -30,3 +30,14 @@ func GetResourceID(u *unstructured.Unstructured) (string, error) { | |||
} | |||
return resourceID, nil | |||
} | |||
|
|||
func GetLocation(u *unstructured.Unstructured) (string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
[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
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
…acpana/dr-beta enh: towards DataformRepository beta
This is a direct, alpha resource. Patch includes: