Skip to content
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

[feature] Write octodns-sync plan to pull request comment #36

Merged
merged 7 commits into from
Jan 17, 2021
Merged

Conversation

solvaholic
Copy link
Owner

@solvaholic solvaholic commented Jan 11, 2021

fixes #35

How to test with the changes in this pull request

Use solvaholic/octodns-sync@issue35 to run the changes pushed to branch issue35:

  • Add an html output to your plan_outputs in your octodns config, for example in public.yaml:

    manager:
      plan_outputs:
        html:
          class: octodns.provider.plan.PlanHtml
  • Configure your workflow to enable add_pr_comment and provide a token:

    on:
      pull_request:
    jobs:
      test:
        runs-on: ubuntu-latest
        steps:
          - uses: actions/checkout@v2
          - uses: solvaholic/octodns-sync@issue35
            with:
              config_path: public.yaml
              add_pr_comment: 'Yes'
              pr_comment_token: '${{ github.token }}'

To-do list

  • Document feature in README
  • Document changes in CHANGELOG
  • Save octodns-sync log and plan output to two separate files
  • [ ] Expose those ☝️ file paths as action outputs
  • Add action inputs to enable feature and to define an API token
  • Add octodns-sync plan as a comment to the triggering pull request when:
    • GITHUB_EVENT_TYPE is pull_request
    • Input add-pr-comment is "Yes"
  • What changes are required for run local option?
  • Provide example configs for different usage scenarios?

and document its use in README.md
in addition to the output log.

This assumes, as was observed in manual testing, the PlanHtml output
will go to stdout whereas the other octodns-sync output will go to
stderr.
for when add_pr_comment is enabled
@solvaholic solvaholic self-assigned this Jan 11, 2021
@solvaholic
Copy link
Owner Author

Expose those ☝️ file paths as action outputs

I changed my mind, would like to do this step in a separate pull request. It's unnecessary for this feature and it'll require changes to entrypoint.sh (and, therefore, the Docker image).

Comment on lines +67 to +75
# Post the comment
_user="fakename" \
_token="${PR_COMMENT_TOKEN}" \
_body="${_body}" \
GITHUB_EVENT_PATH="${GITHUB_EVENT_PATH}" \
python3 -c "import requests, os, json
comments_url = json.load(open(os.environ['GITHUB_EVENT_PATH'], 'r'))['pull_request']['comments_url']
response = requests.post(comments_url, auth=(os.environ['_user'], os.environ['_token']), json={'body':os.environ['_body']})
print(response)"
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For posterity: This block looks the way it does, here, because it was the first way I found to meet these constraints:

  • Use the add an issue comment API with a provided personal access token
  • Get $_body into the JSON payload ~safely
  • Do not install gh or curl
  • Do not introduce dependencies, for example an external Python script or package

and add changes to CHANGELOG.md
@solvaholic
Copy link
Owner Author

What changes are required for run local option?

None.

Provide example configs for different usage scenarios?

I added to README.md to cover available features.

, and fix a nit in README
@solvaholic
Copy link
Owner Author

I'll go ahead and merge this into main. Manual tests went fine and the new feature is disabled by default.

@solvaholic solvaholic merged commit 03f3774 into main Jan 17, 2021
@solvaholic solvaholic deleted the issue35 branch January 18, 2021 02:28
@solvaholic solvaholic restored the issue35 branch January 18, 2021 02:28
@xt0rted
Copy link
Contributor

xt0rted commented Feb 14, 2021

@solvaholic are you planning on doing a new release with these changes or are you waiting until the feedback I had is addressed? Right now I'm pinning to a4b4942 and did my first deploy Friday. It worked great, thanks again for the great action.

@solvaholic solvaholic deleted the issue35 branch May 11, 2021 01:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add plan results to PR as a comment
2 participants