Update Github Actions support #227

TimoRoth · 2020-06-09T13:19:48Z

Github Actions, and the coveralls side of support for it, seem to have changed/evolved.
This PR brings coveralls-python in line with the official github action, which is commonly used to finish parallel builds.

Additionally, there is no need to manually provide your repo token anymore (though you still can, so existing setups should not be affected by this), since the native github support takes the automatically provided GITHUB_TOKEN instead, very similar to how things work on Travis.
You do however have to export that GITHUB_TOKEN to an env var in your workflow.yml:

env:
  GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

TimoRoth · 2020-06-09T13:48:03Z

Not sure what to do about that coverage change. The dip seems unrelated to these changes?

TimoRoth · 2020-06-09T17:45:22Z

While this appears to be working fine, one issue with it is that it spams the Commit-Status with A TON of results, for every individual partial job.
For example:
OGGM/oggm#1033

They are also all red, because they are below the configured minimum percentage.

But it really only seems to work properly for a parallel build when the flag-name is set. Otherwise it ignores all but one parallel steps.

TheKevJames

@TimoRoth thanks for the contribution, much appreciated!

TheKevJames · 2020-06-29T18:23:11Z

coveralls/api.py

+        service = self.config.get('service_name') or ''
+        if self._token_required and service.startswith('github'):
+            gh_token = os.environ.get('GITHUB_TOKEN')
+            if gh_token:
+                self.config['repo_token'] = gh_token
+                return
+            raise CoverallsException(
+                'Running on Github Actions but GITHUB_TOKEN is not set. '
+                'Add "env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}" to '
+                'your step config.')


I think moving this block into load_config_from_github() would help us keep things more encapsulated by service.

TheKevJames · 2020-06-29T18:24:53Z

coveralls/api.py


    @staticmethod
    def load_config_from_github():
-        service_number = os.environ.get('GITHUB_SHA')
+        service = 'github'
+        if 'COVERALLS_REPO_TOKEN' in os.environ:


Could you modify this line to be if os.environ.get('COVERALLS_REPO_TOKEN'):? I've run into some systems before where foo in os.environ always returns True for unset environment variables (eg. they get set to os.environ[foo] = None).

TheKevJames · 2020-06-29T18:26:19Z

coveralls/api.py

+            if gh_token:
+                self.config['repo_token'] = gh_token
+                return
+            raise CoverallsException(


Just to be clear with your top-level comment: what happens in the case when GITHUB_TOKEN is unset but COVERALLS_REPO_TOKEN has been provided? Should this check for either-or?

Then load_config_from_environment sets config['repo_token'] to it and ensure_token bails out from its very first line checking for that.
Will address the rest tomorrow.

Sorry, I should have been more clear, what I'd meant to ask was: is that Ok from Github-Actions' perspective? It sounds like both cases work (ie. either token can be used), just want to be sure.

That depends on the service-name sent to Coveralls it seems. If it's 'github', it can be the Github-Token, if it's not 'github', i.e. the so far used name of 'github-actions', it cannot be the Github-Token.
But the behavior here is wholly undocumented, and just based on testing and guesswork.

Gotcha, thanks for clarifying!

TimoRoth · 2020-07-02T00:02:49Z

I'm not sure why the coverage dropped, and if that's related to these changes at all.
But everything else seems generally.

MarvinT · 2020-07-03T18:03:01Z

These changes solve my issues. What's blocking this from getting merged?

liskin · 2020-07-05T10:46:25Z

coveralls/api.py

+                    'your step config.')
+            self.config['repo_token'] = gh_token
+
+        job = os.environ.get('GITHUB_RUN_NUMBER')


I suggest to use None here instead. GITHUB_RUN_NUMBER is not unique at all, but job is being passed to service_job_id which should be A unique identifier of the job on the service specified by service_name. But GITHUB_RUN_NUMBER isn't unique: A unique number for each run of a particular workflow in a repository. This number begins at 1 for the workflow's first run, and increments with each new run. This number does not change if you re-run the workflow run., it's just a number of the workflow run, it doesn't change for the individual parallel job runs. And there doesn't seem to be any other variable that does identify the individual job run, so our only option is not to pass service_job_id at all and let coveralls generate it itself. I tested that and it works that way.

liskin · 2020-07-05T12:58:24Z

docs/usage/tox.rst

@@ -68,6 +68,9 @@ All variables:
 - ``GITHUB_REF``
 - ``GITHUB_SHA``
 - ``GITHUB_HEAD_REF``
+- ``GITHUB_RUN_ID``
+- ``GITHUB_RUN_NUMBER``


Don't forget to remove this (and from the tests as well) if we confirm that it really is unnecessary. Thanks! :-)

TimoRoth · 2020-07-05T18:55:13Z

I decided that their official action was just broken, and implemented the functionality myself.
This appears to work fine. You do however still have to set the flag name. Otherwise multiple runs of the same Workflow pile up more and more jobs.

liskin · 2020-07-06T09:20:28Z

@TimoRoth Where are you testing this? I tried replacing the official action with a curl and the result was still the same: no commit status from coveralls. :-(

  coveralls-finished:
    needs: tests
    runs-on: ubuntu-latest
    steps:
      - name: Finished
        run: |
          set -ex -o pipefail
          curl -v https://coveralls.io/webhook -d "repo_token=${GITHUB_TOKEN}&repo_name=${GITHUB_REPOSITORY}&payload[build_num]=${GITHUB_RUN_ID}&payload[status]=done"
        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

liskin · 2020-07-06T11:57:26Z

It seems my issue with no commit status was caused by me not having "claimed" the repo on coveralls. After doing that, it seems to work fine even with the official github action (which does the same thing as my curl). I'm still not setting the flag name to prevent commit statuses of individual coverage runs, I don't mind the piling up of jobs: that only happens if I manually re-run the workflow, which I only ever do for intermittent test failures, and these should be rare anyway.

liskin · 2020-07-06T12:20:53Z

coveralls/api.py

+                    'your step config.')
+            self.config['repo_token'] = gh_token
+
+        number = os.environ.get('GITHUB_RUN_NUMBER')


This should really be GITHUB_RUN_ID in case anyone wants multiple different github actions workflows reporting to coveralls. Also, using GITHUB_RUN_ID makes it possible to use https://github.com/marketplace/actions/coveralls-github-action to finalize the parallel build.

Basically, neither of the two is unique enough to be used as job, but GITHUB_RUN_ID is better to identify the build (workflow run), as it's used in github actions URLs, etc.

TimoRoth · 2020-07-06T12:37:05Z

I opted for the RUN_NUMBER because it gives way more meaningful numbers on the builds, rather than super large mostly arbitrary IDs.
But yeah, if you have multiple independent workflows submitting coverage, they'd inevitably collide at some point.

Makes me wonder if setting the build_number to something like "${GITHUB_WORKFLOW}-${GITHUB_RUN_NUMBER}" would be nicer.

liskin · 2020-07-06T12:42:03Z

Makes me wonder if setting the build_number to something like ${GITHUB_WORKFLOW}-${GITHUB_RUN_NUMBER}" would be nicer.

Something like workflow-#123 might be nicer, but I'd still prefer to keep compatibility with the official action performing parallel-finished. Especially if this is purely cosmetic.

TimoRoth · 2020-07-06T12:46:46Z

The official action is clearly broken, given that it submits GITHUB_RUN_ID as service_job_id, which makes no sense if the job_id is really supposed to be unique for the individual jobs.
Which is why I decided to ignore that action and implemented "coveralls --finish".

Primary motivation for this is speed: GitHub Actions doesn't limit the number of concurrent jobs to 4, and also provides a docker registry (GitHub Packages) that we can use to cache the image (building the image takes cca 5 minutes, fetching it would take less than 10 seconds). This is done in another commit. The workflow definition is a bit more complicated because coveralls support for GitHub Actions is less mature than for Travis CI, so we need to manually tell coveralls that all parallel builds have finished and that it can publish the combined result. Also, coveralls-python support is work-in-progress (TheKevJames/coveralls-python#227) so I've tagged a working snapshot of that PR in my fork. Once it's in a released version of coveralls-python I'll switch to it.

TheKevJames · 2020-07-07T00:35:53Z

@TimoRoth thanks for the awesome contribution -- this is looking pretty good to me; I'm going to merge it, run a few more tests, and get it released right away. Really appreciate all the work put into this! Getting --finish implemented will make life a bit simpler :)

@liskin thanks for volunteering some code review time, much appreciate the extra eyes on this!

@MarvinT no blockers, just needed to find some time in my day for OSS work. Sorry for any delays!

TheKevJames · 2020-07-07T00:49:03Z

This is now live on PyPI as v2.1.0 -- Conda release will happen shortly.

TheKevJames/coveralls-python#227 was merged

* add coverage configuration * yaml whitespace fixes * remove coveralls app and use python-coveralls * use full conda path to call coveralls * test-requirements.txt * CI_BRANCH env var * GITHUB_TOKEN env variable * try python-coveralls PR? * pip install -e . * Update test-requirements.txt TheKevJames/coveralls-python#227 was merged * Update test-requirements.txt * fix typo * -e * add badge Co-authored-by: ajlee21 <alexjlee.21@gmail.com>

- Align behavior with coveralls-python (see [1]) - Provide `service_number` if run on GitHub Actions (see [2]) - Add support for custom `flag_name` - Provide `service_pull_request` if possible [1] https://github.com/coveralls-clients/coveralls-python/blob/30e4815169b3db2616981939d55d2f4495816821/coveralls/api.py#L73-L146 [2] TheKevJames/coveralls-python#227

The environment varialbe that needs to set seems to have changed as of this PR [1] [1] TheKevJames/coveralls-python#227

The environment variable that needs to set seems to have changed as of [this PR][1] [1]: TheKevJames/coveralls-python#227

TimoRoth requested a review from TheKevJames as a code owner June 9, 2020 13:19

TimoRoth force-pushed the master branch 4 times, most recently from 94044ef to 29f84a9 Compare June 9, 2020 13:36

TimoRoth force-pushed the master branch 4 times, most recently from 4094f70 to 8904b1a Compare June 9, 2020 15:54

TimoRoth force-pushed the master branch 3 times, most recently from 46d8ad3 to dd87892 Compare June 9, 2020 18:52

TheKevJames requested changes Jun 29, 2020

View reviewed changes

TimoRoth force-pushed the master branch 3 times, most recently from 031b9bb to 4b11312 Compare July 1, 2020 23:52

MarvinT mentioned this pull request Jul 3, 2020

Test Code Coverage greenelab/ponyo#11

Merged

This was referenced Jul 3, 2020

Using for Python code and coverage.py coverallsapp/github-action#4

Closed

Add service_number for github actions #216

Closed

Can I use only LCOV files? coverallsapp/github-action#30

Open

liskin reviewed Jul 5, 2020

View reviewed changes

TimoRoth force-pushed the master branch from 4b11312 to 4f99e18 Compare July 5, 2020 12:49

liskin reviewed Jul 5, 2020

View reviewed changes

Fix tests not expexting to actually run on GitHub actions

5224448

TimoRoth force-pushed the master branch 2 times, most recently from 6697680 to 92775f1 Compare July 5, 2020 14:41

TimoRoth force-pushed the master branch 6 times, most recently from e929368 to 9068caa Compare July 5, 2020 18:03

liskin reviewed Jul 6, 2020

View reviewed changes

TimoRoth force-pushed the master branch from 9068caa to 28ae96d Compare July 6, 2020 13:08

TimoRoth added 2 commits July 6, 2020 15:10

Update Github Actions support

aa1ca0c

Add cli option to finish parallel build

34afc2f

TimoRoth force-pushed the master branch from 28ae96d to 34afc2f Compare July 6, 2020 13:11

TheKevJames merged commit f597109 into TheKevJames:master Jul 7, 2020

TheKevJames mentioned this pull request Jul 7, 2020

DEBUG: print $GITHUB_EVENT_NAME #225

Closed

MarvinT added a commit to MarvinT/ponyo that referenced this pull request Jul 9, 2020

Update test-requirements.txt

84157a4

TheKevJames/coveralls-python#227 was merged

nils-braun mentioned this pull request Jul 18, 2020

Change the coveralls github action to fix missing coveralls in PRs blue-yonder/tsfresh#734

Merged

vamega added a commit to vamega/Box that referenced this pull request Oct 27, 2020

Fix coveralls.

a9dcbe8

The environment varialbe that needs to set seems to have changed as of this PR [1] [1] TheKevJames/coveralls-python#227

vamega mentioned this pull request Oct 27, 2020

Fix coveralls. cdgriffith/Box#173

Merged

vamega added a commit to vamega/Box that referenced this pull request Oct 27, 2020

Fix coveralls.

e3ba3d7

The environment variable that needs to set seems to have changed as of [this PR][1] [1]: TheKevJames/coveralls-python#227

liskin mentioned this pull request Jan 14, 2021

Github Actions raises a 422 error #252

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Github Actions support #227

Update Github Actions support #227

TimoRoth commented Jun 9, 2020

TimoRoth commented Jun 9, 2020

TimoRoth commented Jun 9, 2020

TheKevJames left a comment

TheKevJames Jun 29, 2020

TheKevJames Jun 29, 2020

TheKevJames Jun 29, 2020

TimoRoth Jun 30, 2020

TheKevJames Jun 30, 2020

TimoRoth Jun 30, 2020

TheKevJames Jul 1, 2020

TimoRoth commented Jul 2, 2020

MarvinT commented Jul 3, 2020

liskin Jul 5, 2020

liskin Jul 5, 2020 •

edited

Loading

TimoRoth commented Jul 5, 2020

liskin commented Jul 6, 2020

liskin commented Jul 6, 2020

liskin Jul 6, 2020

TimoRoth commented Jul 6, 2020

liskin commented Jul 6, 2020

TimoRoth commented Jul 6, 2020

TheKevJames commented Jul 7, 2020

TheKevJames commented Jul 7, 2020

Update Github Actions support #227

Update Github Actions support #227

Conversation

TimoRoth commented Jun 9, 2020

TimoRoth commented Jun 9, 2020

TimoRoth commented Jun 9, 2020

TheKevJames left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TimoRoth commented Jul 2, 2020

MarvinT commented Jul 3, 2020

Choose a reason for hiding this comment

liskin Jul 5, 2020 • edited Loading

Choose a reason for hiding this comment

TimoRoth commented Jul 5, 2020

liskin commented Jul 6, 2020

liskin commented Jul 6, 2020

Choose a reason for hiding this comment

TimoRoth commented Jul 6, 2020

liskin commented Jul 6, 2020

TimoRoth commented Jul 6, 2020

TheKevJames commented Jul 7, 2020

TheKevJames commented Jul 7, 2020

liskin Jul 5, 2020 •

edited

Loading