Add gh-action docs ♨️🥕 by lilyminium · Pull Request #3051 · MDAnalysis/mdanalysis

lilyminium · 2020-12-02T07:39:27Z

Fixes #3046

Changes made in this Pull Request:

Changes deployment scripts to get run by gh-actions
This could likely be neater and nicer (e.g. not exporting all the environment variables), but I think these are the minimal changes required.

Built dev / 2.0.0 docs here

Targeted to #3040 but could target to develop instead of we prefer to keep this separate.

PR Checklist

Tests?
Docs?
CHANGELOG updated?
Issue raised/referenced?

This reverts commit eaf7051.

codecov · 2020-12-02T09:44:26Z

Codecov Report

Merging #3051 (7e9737c) into gh-actions-ci (85b1576) will decrease coverage by 3.23%.
The diff coverage is n/a.

@@                Coverage Diff                @@
##           gh-actions-ci    #3051      +/-   ##
=================================================
- Coverage          93.09%   89.85%   -3.24%     
=================================================
  Files                186      174      -12     
  Lines              24665    23123    -1542     
  Branches            3196        0    -3196     
=================================================
- Hits               22961    20778    -2183     
- Misses              1656     2345     +689     
+ Partials              48        0      -48

Impacted Files	Coverage Δ
package/MDAnalysis/topology/RDKitParser.py	`15.25% <0.00%> (-81.36%)`	⬇️
package/MDAnalysis/coordinates/RDKit.py	`16.47% <0.00%> (-80.85%)`	⬇️
package/MDAnalysis/analysis/hole2/hole.py	`14.47% <0.00%> (-58.72%)`	⬇️
package/MDAnalysis/analysis/hole2/utils.py	`25.78% <0.00%> (-50.32%)`	⬇️
package/MDAnalysis/analysis/psa.py	`79.14% <0.00%> (-10.14%)`	⬇️
package/MDAnalysis/analysis/align.py	`92.08% <0.00%> (-5.76%)`	⬇️
package/MDAnalysis/core/universe.py	`92.94% <0.00%> (-4.87%)`	⬇️
package/MDAnalysis/coordinates/H5MD.py	`91.08% <0.00%> (-3.47%)`	⬇️
package/MDAnalysis/core/selection.py	`95.93% <0.00%> (-3.32%)`	⬇️
package/MDAnalysis/lib/util.py	`89.47% <0.00%> (-0.59%)`	⬇️
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 85b1576...7e9737c. Read the comment docs.

IAlibay

Overall lgtm, just a couple of questions.

I saw some commits linking to your own repo, I'm assuming that was testing & it works?

IAlibay · 2020-12-02T13:13:32Z

.github/workflows/gh-ci.yaml

      run: |
-        # place the deploy call here
-        echo "Oh, maple syrup roast parsnips [Richard Gowers]"
+        python -c 'import os,sys,fcntl; flags = fcntl.fcntl(sys.stdout, fcntl.F_GETFL); fcntl.fcntl(sys.stdout, fcntl.F_SETFL, flags&~os.O_NONBLOCK);'


I know this has been around forever, but mostly out of curiosity, what does it actually do?

I think there used to be a comment somewhere... related to hanging builds? @lilyminium had explained it to me and I promptly forgot.

IAlibay · 2020-12-02T13:16:10Z

.github/workflows/gh-ci.yaml

+        cd package/MDAnalysis
+        export VERSION=$(python -c "import version; print(version.__version__)")
+        cd -
+        export GH_DOC_BRANCH=$GITHUB_REF


not that it really matters (especially since I think you still need to do the cd - either way), but these could be all put inside an env: block if you happened to think it was cleaner that way.

IAlibay · 2020-12-02T13:41:37Z

maintainer/deploy_docs.sh

-git config user.name "${GIT_CI_USER}"
-git config user.email "${GIT_CI_EMAIL}"
+git config user.name github-actions
+git config user.email github-actions@github.com


I can't find the gh actions details on this, do you have a reference for these? I keep finding "github-action@users.noreply.github.com", but that's in random github repos :/

Do you think it matters? Didn't we use in the past "TravisCI@mdanlysis.org" – if anyone wants the commits associated with their name then they could put in theirs, e.g. lilyminium@users.noreply.github.com.

I guess probably not? This whole area of using gh actions to push to repos just seems sparingly documented (unless I'm missing a really important page), particularly when it comes to examples. But I guess you're right, git doesn't really care if the email address is real.

@IAlibay you're right I do see the noreply credentials more often; I'm not sure where I stole the original from.

orbeckst

If it works then it's all cool.

See comments for potential improvements.

I'd be ok to have it as a PR against develop as opposed to increase #3040 . EDIT: ... up to you and @IAlibay

orbeckst · 2020-12-02T18:09:10Z

.github/workflows/gh-ci.yaml

+        export VERSION=$(python -c "import version; print(version.__version__)")
+        cd -
+        export GH_DOC_BRANCH=$GITHUB_REF
+        export GH_TOKEN=${{ secrets.GITHUB_TOKEN }}


Does GH-actions obscure encrypted tokens in the logs?

Is this the old oauth2 token or a new one?

The secrets.GITHUB_TOKEN is automatically created by GH (edit: so not the old oauth2 token). But, no, Github does not redact secrets if they are printed.

So yeah this could be an issue if someone is really determined to get it and knows how to google "how to see git secrets"

Edit: https://github.bokerqi.topmunity/t/how-to-see-my-git-secrets/123668/7 actually let's just not merge anything that prints it

Update: Yes, you can echo {{ secrets.GITHUB_TOKEN }}, but it's a) single-use and b) read-only on forked repositories. This PR from my alt account fails to write to lilyminium/mdanalysis:gh-pages with a permission error.

orbeckst · 2020-12-02T18:10:40Z

.github/workflows/gh-ci.yaml

+        cd package/MDAnalysis
+        export VERSION=$(python -c "import version; print(version.__version__)")
+        cd -
+        export GH_DOC_BRANCH=$GITHUB_REF


Every env var (except the oauth2 token) could also become a commandline argument for the deploy_docs.sh script. However, as long as it's working the way it is I don't see much need to change it.

I don't like those because I can never remember which order things are in 😅 Regardless it's probably better than cluttering up the environment. I think I will actually take @IAlibay's suggestion and move things to env / move the whole deployment within gh-ci.yaml, unless there are objections; the general pros for having a separate script (modularity, being able to deploy docs outside of CI) are somewhat negated by the heavy dependence on environment variables set in the CI.

orbeckst · 2020-12-02T18:13:44Z

maintainer/deploy_docs.sh

 mv ../${VERSION} $VERSION

-git remote add upstream "https://${GH_TOKEN}@${GH_REPOSITORY}"
+git remote add upstream "https://github-actions:${GH_TOKEN}@${GH_REPOSITORY}"


A user name is needed now?

Does it in any way relate to the user.name config?

If this is the case then I would make the script more robust by also passing the name (as env var or commandline argument); I would avoid having hard-coded dependencies between the gh-actions script and the deploy script.

IAlibay · 2020-12-02T18:25:57Z

I'd be ok to have it as a PR against develop as opposed to increase #3040 . EDIT: ... up to you and @IAlibay

I don't mind, I don't think it really matters either way. If #3040 is ready to merge now (was there anything else left @orbeckst?), then I'd say let's just go ahead and squash merge that one and then have this one target develop. My reasons mainly being a) if for some reason we don't merge this soon-ish, at least other PRs can be opened b) we could then more cleanly introduce a small change to the docs here and check that it worked as intended?

orbeckst · 2020-12-02T18:49:46Z

I agree with you, @IAlibay : merge #3040 and then add this on ~~top~~ EDIT develop.

(We will have to backport both to master, right? But that's another 🍯 of 🐟 )

IAlibay · 2020-12-02T18:51:57Z

I agree with you, @IAlibay : merge #3040 and then add this on ~~top~~ EDIT develop.

(We will have to backport both to master, right? But that's another 🍯 of 🐟 )

Yup, it's on my todo list to open the master PR next :) I'd like to get 1.0.1 out there asap.

IAlibay · 2020-12-02T18:59:07Z

Sorry I accidentally closed by deleting the gh-actions-ci branch 😱 I've re-added it for now just in case.

lilyminium · 2020-12-03T07:37:29Z

Superseded by #3053.

add doc deployment

5847f40

lilyminium requested review from IAlibay and orbeckst December 2, 2020 07:39

lilyminium force-pushed the gh-docs branch from 7219aaf to 044b0be Compare December 2, 2020 07:54

rename doc script

0a0d659

lilyminium force-pushed the gh-docs branch 3 times, most recently from ed794c2 to 3b32f95 Compare December 2, 2020 08:10

to undo: change site to lilyminium's

eaf7051

lilyminium force-pushed the gh-docs branch from 3b32f95 to eaf7051 Compare December 2, 2020 08:17

Revert "to undo: change site to lilyminium's"

7e9737c

This reverts commit eaf7051.

lilyminium mentioned this pull request Dec 2, 2020

Add stable/ and dev/ versions of UserGuide MDAnalysis/UserGuide#110

Closed

IAlibay approved these changes Dec 2, 2020

View reviewed changes

orbeckst approved these changes Dec 2, 2020

View reviewed changes

orbeckst mentioned this pull request Dec 2, 2020

Adds basic GH Actions CI workflow #3040

Merged

4 tasks

IAlibay closed this Dec 2, 2020

IAlibay deleted the branch MDAnalysis:gh-actions-ci December 2, 2020 18:56

IAlibay reopened this Dec 2, 2020

lilyminium mentioned this pull request Dec 3, 2020

Deploy docs via GitHub Actions CI #3053

Merged

4 tasks

lilyminium closed this Dec 3, 2020

Conversation

lilyminium commented Dec 2, 2020

PR Checklist

Uh oh!

codecov bot commented Dec 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

IAlibay left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

orbeckst left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lilyminium Dec 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lilyminium Dec 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IAlibay commented Dec 2, 2020

Uh oh!

orbeckst commented Dec 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

IAlibay commented Dec 2, 2020

Uh oh!

IAlibay commented Dec 2, 2020

Uh oh!

lilyminium commented Dec 3, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Dec 2, 2020 •

edited

Loading

orbeckst left a comment •

edited

Loading

lilyminium Dec 2, 2020 •

edited

Loading

lilyminium Dec 2, 2020 •

edited

Loading

orbeckst commented Dec 2, 2020 •

edited

Loading