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

Comment on pull requests with GitHub Actions #322

Open
dhimmel opened this issue Mar 10, 2020 · 16 comments
Open

Comment on pull requests with GitHub Actions #322

dhimmel opened this issue Mar 10, 2020 · 16 comments

Comments

@dhimmel
Copy link
Member

dhimmel commented Mar 10, 2020

Currently GitHub actions stores artifacts with rendered manuscript for pull request builds. However, Actions do not leave comments on pull requests. AppVeyor does leave comments, but requires using an additional CI service and duplicating computation.

Therefore, it'd simplify things if we could have GitHub Actions comment on PR, perhaps as an optional feature. Users such as @olgabot and @agitter have indicated they like the convenience of PR comments. PR comments are also good for users that don't have a deep understand of CI and are better suited with ChatOps style interfaces.

It looks like there are already PR comment actions:

Related issues that we've addressed:

@dhimmel
Copy link
Member Author

dhimmel commented Mar 10, 2020

Here is what an example comment could look like:


Actions build 483333557 for commit a0b98d3 by XXXXX is now complete. The rendered manuscript from this build is temporarily available.

see build log
2020-03-03T20:50:01.5475507Z Retrieving and processing reference metadata
2020-03-03T20:50:01.9723062Z ## INFO
2020-03-03T20:50:01.9723348Z Manuscript content parts:
2020-03-03T20:50:01.9724328Z 00.front-matter
2020-03-03T20:50:01.9725960Z 01.abstract
2020-03-03T20:50:01.9726432Z 02.delete-me
2020-03-03T20:50:01.9726851Z 90.back-matter
2020-03-03T20:50:01.9727064Z ## INFO
2020-03-03T20:50:01.9727651Z no citation tags file at content/citation-tags.tsv Not reading citekey_aliases from citation-tags.tsv.
2020-03-03T20:50:01.9753380Z ## INFO
2020-03-03T20:50:01.9753634Z Using UTC timezone.
2020-03-03T20:50:01.9754613Z Dating manuscript with the current datetime: 2020-03-03T20:50:01.974742+00:00
2020-03-03T20:50:02.1175352Z ## INFO
2020-03-03T20:50:02.1176198Z get_rootstock_commit added a `rootstock` remote to the git repository.
2020-03-03T20:50:02.3530287Z ## INFO
2020-03-03T20:50:02.3530881Z Generated manscript stats:
2020-03-03T20:50:02.3531190Z {
2020-03-03T20:50:02.3531467Z   "word_count": 1395
2020-03-03T20:50:02.3531735Z }
2020-03-03T20:50:02.4351323Z Exporting HTML manuscript
2020-03-03T20:50:02.5972349Z [INFO] Running filter pandoc-fignos
2020-03-03T20:50:02.7590607Z [INFO] Completed filter pandoc-fignos in 18 ms
2020-03-03T20:50:02.7590936Z [INFO] Running filter pandoc-eqnos
2020-03-03T20:50:02.8526978Z 
2020-03-03T20:50:02.8528386Z pandoc-eqnos: Wrote the following blocks to header-includes.  If you
2020-03-03T20:50:02.8528954Z use pandoc's --include-in-header option then you will need to manually
2020-03-03T20:50:02.8529320Z include these yourself.
2020-03-03T20:50:02.8529555Z 
2020-03-03T20:50:02.8530008Z     <!-- pandoc-eqnos: equation style -->
2020-03-03T20:50:02.8530308Z     <style>
2020-03-03T20:50:02.8530777Z       .eqnos { display: inline-block; position: relative; width: 100%; }
2020-03-03T20:50:02.8531088Z       .eqnos br { display: none; }
2020-03-03T20:50:02.8531570Z       .eqnos-number { position: absolute; right: 0em; top: 50%; line-height: 0; }
2020-03-03T20:50:02.8531891Z     </style>
2020-03-03T20:50:02.8846372Z [INFO] Completed filter pandoc-eqnos in 14 ms
2020-03-03T20:50:02.8846671Z [INFO] Running filter pandoc-tablenos
2020-03-03T20:50:03.0090787Z [INFO] Completed filter pandoc-tablenos in 16 ms
2020-03-03T20:50:03.0093357Z [INFO] Running filter pandoc-manubot-cite
2020-03-03T20:50:06.4214735Z [INFO] Completed filter pandoc-manubot-cite in 32 ms
2020-03-03T20:50:06.4219008Z [INFO] Running filter pandoc-citeproc
2020-03-03T20:50:06.5889419Z [INFO] Completed filter pandoc-citeproc in 26 ms
2020-03-03T20:50:06.8003667Z Exporting PDF manuscript using Docker + Athena
2020-03-03T20:50:07.2849361Z Unable to find image 'arachnysdocker/athenapdf:2.16.0' locally
2020-03-03T20:50:07.5496471Z 2.16.0: Pulling from arachnysdocker/athenapdf
2020-03-03T20:50:07.5497726Z cd0a524342ef: Pulling fs layer
2020-03-03T20:50:07.5502989Z 650579e69619: Pulling fs layer
2020-03-03T20:50:07.5513098Z 22bd9d71423b: Pulling fs layer
2020-03-03T20:50:07.5519281Z 3b09cc01502a: Pulling fs layer
2020-03-03T20:50:07.5519886Z e9feb93bc1fb: Pulling fs layer
2020-03-03T20:50:07.5520317Z 92883a0197ad: Pulling fs layer
2020-03-03T20:50:07.5520725Z f4f65809d38d: Pulling fs layer
2020-03-03T20:50:07.5521138Z e9feb93bc1fb: Waiting
2020-03-03T20:50:07.5521546Z 92883a0197ad: Waiting
2020-03-03T20:50:07.5521951Z f4f65809d38d: Waiting
2020-03-03T20:50:07.5522353Z 3b09cc01502a: Waiting
2020-03-03T20:50:07.7069961Z 650579e69619: Verifying Checksum
2020-03-03T20:50:07.7073196Z 650579e69619: Download complete
2020-03-03T20:50:07.8758386Z 3b09cc01502a: Verifying Checksum
2020-03-03T20:50:07.8829080Z 3b09cc01502a: Download complete
2020-03-03T20:50:08.3346754Z cd0a524342ef: Verifying Checksum
2020-03-03T20:50:08.3346899Z cd0a524342ef: Download complete
2020-03-03T20:50:08.4937198Z e9feb93bc1fb: Verifying Checksum
2020-03-03T20:50:08.4937400Z e9feb93bc1fb: Download complete
2020-03-03T20:50:08.5714331Z 92883a0197ad: Verifying Checksum
2020-03-03T20:50:08.5714541Z 92883a0197ad: Download complete
2020-03-03T20:50:08.6656264Z f4f65809d38d: Verifying Checksum
2020-03-03T20:50:08.6674453Z f4f65809d38d: Download complete
2020-03-03T20:50:09.0545128Z 22bd9d71423b: Verifying Checksum
2020-03-03T20:50:09.0545375Z 22bd9d71423b: Download complete
2020-03-03T20:50:10.6072363Z cd0a524342ef: Pull complete
2020-03-03T20:50:10.6592527Z 650579e69619: Pull complete
2020-03-03T20:50:18.4938035Z 22bd9d71423b: Pull complete
2020-03-03T20:50:18.5295006Z 3b09cc01502a: Pull complete
2020-03-03T20:50:19.6564733Z e9feb93bc1fb: Pull complete
2020-03-03T20:50:19.6893449Z 92883a0197ad: Pull complete
2020-03-03T20:50:19.7253131Z f4f65809d38d: Pull complete
2020-03-03T20:50:19.7274863Z Digest: sha256:213203e3b46017d21d8fb08cd056d43272029a963616253fb47948c4af48845c
2020-03-03T20:50:19.7289711Z Status: Downloaded newer image for arachnysdocker/athenapdf:2.16.0
2020-03-03T20:50:27.3555100Z ATTENTION: default value of option force_s3tc_enable overridden by environment.
2020-03-03T20:50:30.1123102Z Converted 'file:///converted/manuscript.html' to PDF: 'manuscript.pdf'
2020-03-03T20:50:30.1131123Z PDF Conversion: 2941.357ms
2020-03-03T20:50:30.3560499Z Build complete

including the output of the build log would be helpful for spotting warnings.

@olgabot
Copy link
Contributor

olgabot commented Mar 11, 2020

This is awesome! Looking forward to unifying all the build rules under GitHub Actions. Some feedback:

  • I think specifying "GitHub Actions" vs "Actions" will be helpful as it is more explicit for people who aren't knee-deep in continuous integration
  • What causes the manuscript to be temporarily available? Is it possible to specify a time frame, e.g. is it temporary for the next 1 min or next 6 months? To me, this seems suboptimal as I personally like going back through previous PDF versions of manuscripts and seeing them grow. Is it possible to upload them to the commit and make them permanent?

@agitter
Copy link
Member

agitter commented Mar 11, 2020

This is great. I have a slight preference for the AppVeyor comments that directly link the manuscript pdf and html instead of zipping them. However, I see benefits to this zip file that has additional outputs that can help with debugging.

I agree with @olgabot that the "GitHub Actions" phrasing is helpful.

I'm okay with pull request artifacts being temporary. @olgabot all of the manuscripts that are built from the master branch and deployed are archived and versioned. For example, historical rootstock manuscripts are in https://github.com/manubot/rootstock/tree/gh-pages/v

@dhimmel
Copy link
Member Author

dhimmel commented Mar 16, 2020

Just heard parts of a great presentation by @abhisuri97 on CI. He mentioned the github-script action, which is able to make comments directly using the GitHub API. So probably a lighter-weight solution than the actions linked to above.

What causes the manuscript to be temporarily available? Is it possible to specify a time frame, e.g. is it temporary for the next 1 min or next 6 months?

It actually seems like GitHub Action artifacts aren't automatically deleted according to actions/upload-artifact#9. Although at some point you may hit a repo artifact quota... not quite sure.

@agitter
Copy link
Member

agitter commented Mar 26, 2020

@dhimmel these GitHub Actions could be quite helpful for the COVID-19 review. I may be able to make some time to work on it over the next few days. Should I start by looking at the github-script action and itemizing a few of the open questions we'll have to answer before we can implement this?

Those questions would mostly pertain to what is available through the GitHub API because I haven't worked with it much.

@dhimmel
Copy link
Member Author

dhimmel commented Mar 26, 2020

Yes feel free to take the lead here and I can answer any questions you may have or help deal with GitHub Issue quirks.

@agitter
Copy link
Member

agitter commented Mar 26, 2020

I did some testing. harupy/comment-on-pr does fit our use case. The comment content is constructed through a template file. That could be a nice approach, but the template file only supports two specific variables. It's not general enough.

unsplash/comment-on-pr and github-script are straightforward to set up. I have both of them working with a message that is set in a previous workflow step and stored in an environment variable.

@dhimmel I'm planning to extend the "Upload Artifacts" step to construct a comment message similar to what you have above and store it in an environment variable. Then, the new comment workflow step would make that comment in the pull request. Is that what you had in mind?

I don't expect this approach would support including the full build log in the comment.

@dhimmel
Copy link
Member Author

dhimmel commented Mar 26, 2020

I'm planning to extend the "Upload Artifacts" step to construct a comment message

You probably want a new step so it can have it's own run command. So there'd be two new steps: "Create comment text" and "Comment on pull request".

Using github-script is probably preferable to unsplash/comment-on-pr. Probably more likely to be maintained longterm, more trustworthy, and possible faster & more versatile as a pure javascript action.

@agitter
Copy link
Member

agitter commented Mar 27, 2020

Getting the artifact URL seems like a dead end. There isn't support for lasting URLs per actions/upload-artifact#60

Even constructing the artifact URL is nowhere near as straightforward as it was with AppVeyor. I don't see the ids in the artifact URL in the GitHub context in the workflow. GitHub-artifact-URL is a Python script that wraps the API to construct the redirect URL. However, the author reports it now required authentication per JayFoxRox/GitHub-artifact-URL#4

Unless I'm missing something, my conclusion is that GitHub Actions artifacts aren't mature enough to support what we want.

@dhimmel
Copy link
Member Author

dhimmel commented Mar 27, 2020

my conclusion is that GitHub Actions artifacts aren't mature enough to support what we want.

And it looks like actions/upload-artifact@v2 won't fix the limitation of not having a stable article download URL as per actions/upload-artifact#62 (comment).

I'd still be interested in you opening a draft PR with your progress. We might want to comment with other information like log output, so hopefully we'll have a chance to use much of this work someday.

@dhimmel
Copy link
Member Author

dhimmel commented Mar 30, 2020

Given that #327 has hit some dead ends, mainly the lack of persistent URLs for artifacts and permissions problems, maybe we should consider another approach. Another option would be a ChatOps approach where a user comments on a PR with @manubot pdf and actions leaves a comment that uploads the PDF.

The COVID-19 issue labeler is somewhat similar.

Or perhaps AppVeyor is the best solution for now.

@agitter
Copy link
Member

agitter commented Mar 30, 2020

actions leaves a comment that uploads the PDF

Does this mean that the PDF would be stored as a file attachment in the comment as a workaround to the artifact URL problem? That would be a clever approach. We'd have to explore how to automate file attachments.

@dhimmel
Copy link
Member Author

dhimmel commented Mar 31, 2020

Does this mean that the PDF would be stored as a file attachment in the comment as a workaround to the artifact URL problem?

Yes, but it looks like attachments on comments via the API are not supported as per isaacs/github#1133.

@olgabot
Copy link
Contributor

olgabot commented Apr 9, 2020

How does JOSS do it? e.g see this comment which links to a PDF on GitHub: openjournals/joss-reviews#1584 (comment)

@olgabot
Copy link
Contributor

olgabot commented Apr 9, 2020

The COVID-19 issue labeler is somewhat similar.

Also this is cool! Will share widely

@dhimmel
Copy link
Member Author

dhimmel commented Apr 9, 2020

How does JOSS do it? e.g see this comment which links to a PDF on GitHub

JOSS runs servers where @whedon lives and responds to mentions. Thus far we've avoiding having to maintain any manubot servers, both for sustainability as well as decentralization. Currently, once you create a manuscript you are no longer dependent on any centralized manubot services. And manubot runs in a serverless manner since all computation is outsourced to GitHub Actions / CI.

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

No branches or pull requests

3 participants