Automation Runbook: support for setting custom content #1696
Conversation
tombuildsstuff
added
enhancement
upstream/microsoft
|
@metacpp this is waiting on a version of the SDK that's at least a few weeks out (to be released & for us to upgrade to) - so I'm de-assigning this PR until we have a path forward for it. |
tombuildsstuff
removed
the
upstream/microsoft
tombuildsstuff
force-pushed
the
automation_publish
branch
from
2b2bb0c
to
ba12631
Compare
There was a problem hiding this comment.
hey @hbuckle
Thanks for this PR - I've taken a look through and left a couple of minor comments (which I'm going to push a commit to fix, I hope you don't mind!) - but this otherwise LGTM 👍
Thanks!
| if err != nil { | ||
| return err | ||
| } | ||
| if contentBytes := *response.Value; contentBytes != nil { |
There was a problem hiding this comment.
there's a crash here if response.Value is nil
azurerm/config.go
Outdated
| runbookDraftClient.Authorizer = auth | ||
| runbookDraftClient.Sender = sender | ||
| runbookDraftClient.SkipResourceProviderRegistration = c.skipProviderRegistration | ||
| c.automationRunbookDraftClient = runbookDraftClient |
There was a problem hiding this comment.
can we switch this over to using c.configureClient(&runbookDraftClient.Client, auth)
| } | ||
| _, err = draftClient.Publish(ctx, resGroup, accName, name) | ||
| if err != nil { | ||
| return err |
There was a problem hiding this comment.
can we wrap this errors to explain why these failures occurred?
tombuildsstuff
changed the title
|
Tests pass:
|
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks! |
ghost
locked and limited conversation to collaborators
Adds support for directly setting the content of a runbook.
This will require the (as yet unreleased) v20 of the azure-sdk-for-go to resolve this issue, so we need to wait for that before merging.