-
Notifications
You must be signed in to change notification settings - Fork 17
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
Add plan results to PR as a comment #35
Comments
I like this idea, thank you @xt0rted ! To be sure I'm thinking clearly about what you're after.. Would it meet your needs to add an octodns-sync input like
And, when that's set, octodns-sync adds the HTML-formatted plan comment to the PR that triggered the action, like in the example you provided? Adding a comment will be straightforward, with |
Yea, I figured making this opt-in would be the best way to do it. It's been awhile since I played with octodns but from what I recall the code in that issue worked fine for me back when it was first posted. At the time I was running it locally, I never got far enough to deploy it so hopefully this is even easier now with actions. The other settings that might be useful are one for the api token and the name & email of the account leaving the comment. |
10-4, thank you for confirming.
👍, adding the PR comment may need a token. Would you use the name and email for attributing the comment? or something else?
After adding this to manager:
plan_outputs:
html:
class: octodns.provider.plan.PlanHtml This command: octodns-sync --config-file="/config/public.yaml" 2>/dev/null Produced this output: example.com.route53
|
I'm thinking:
This shell script: #!/bin/bash
_needle='<!--- octodns/plan --->'
_header="## octoDNS Plan for {sha}"
_footer="Automatically generated by [solvaholic/octodns-sync](https://github.com/solvaholic/octodns-sync)"
_config_path="/config/public.yaml"
_planfile="/octodns-sync.plan"
_logfile="/octodns-sync.log"
echo "${_needle}" > "${_planfile}"
echo " " >> "${_planfile}"
echo "${_header}" >> "${_planfile}"
echo " " >> "${_planfile}"
script "${_logfile}" -e -c "octodns-sync --config-file=\"${_config_path}\" >>\"${_planfile}\""
echo " " >> "${_planfile}"
echo "${_footer}" >> "${_planfile}" Produces the output similar to the example: octoDNS Plan for {sha}winans.net.route53
Automatically generated by solvaholic/octodns-sync |
After looking this over again the name & email options aren't needed since there's no commit activity. The reason I mentioned it is because I try to keep automated activity associated with the I like the idea of saving the plan output. Can this be saved as json or yaml? That could make it even more useful if you wanted to work with the data in a later step. For now I just want a comment on the PR, but down the road I might want to post to slack the changes that were just applied. I don't want to ask for features I may never use though. |
Cool, thank you for explaining @xt0rted
I think that functionality could definitely be added. The plan output is produced by octodns's plan.py, which currently has Plans for Logger, Markdown, and Html. As long as the sync command puts the output you're looking for on stdout, it'll be saved as the plan output.
I appreciate your requests, and your consideration 🙇 As of d42b3f1 the
If you have a chance to try it out, please let me know what you think. |
@xt0rted this feature is available and documented in Please let me know what questions or feedback you have. 🙇 |
I'm just now testing this update and have a few issues I'm running into. The first thing I noticed was if there are errors running OctoDNS a comment is still left on the pr. I don't think this is really an issue, it's just not what I expected since OctoDNS didn't successfully run. I figured this step would throw an error and the workflow would end here. My initial run had the wrong api credentials which resulted in the following log output:
And this is the comment that was left: The second thing is each time the action runs it adds a new comment to the PR which can get very noisy. The logger in octodns/octodns#156 (comment) checks the PR comments for one with
|
Fantastic, thank you for letting me know @xt0rted 🙇 I'll set up new issues for these now. |
Is your enhancement request related to a problem? Please describe.
Right now you have to look through the log output to see what the proposed changes are, it'd be a lot nicer if this could be added to the PR as a comment for easier review.
Describe the solution you'd like
I'd like to see a plan logger that comments on the PR, if being run from one, with a table of the proposed changes. There's a version of this in the octoDNS repo which can be found in octodns/octodns#156 (comment) along with examples of what the comment looks like.
Describe alternatives you've considered
Creating my own action/scripts which I'd rather not do since I've never worked with python.
Additional context
The text was updated successfully, but these errors were encountered: