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

Fix 'publish_TestPyPI' CI job #337

Conversation

CasperWA
Copy link
Member

@CasperWA CasperWA commented Jun 12, 2020

Fixes #331

Use github.event_name instead of github.event.
See the difference here.

Tested this in my fork, it ran for a push to my master branch (which is the top CI run requirement), see the action run here.
Never mind that it fails, this is due to my fork not having the correct secret.

The last two tests that this works lies in making sure that:

  • The job 'publish_TestPyPI' is skipped for this PR, and
  • The job is not skipped when merging this in and that it succeeds (naturally).

@CasperWA CasperWA requested review from ml-evs and shyamd June 12, 2020 12:29
@CasperWA
Copy link
Member Author

Note: Another solution to this would be to factor out the 'publish_TestPyPI' into a separate CI yml file, which only runs for pushes into 'master'?

This is essentially how we used to do it, and is a way I would prefer, since the current setup introduces some obscurity.

@codecov
Copy link

codecov bot commented Jun 12, 2020

Codecov Report

Merging #337 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #337   +/-   ##
=======================================
  Coverage   90.56%   90.56%           
=======================================
  Files          55       55           
  Lines        2268     2268           
=======================================
  Hits         2054     2054           
  Misses        214      214           
Flag Coverage Δ
#unittests 90.56% <ø> (ø)

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 6c51f34...2f69a4f. Read the comment docs.

@shyamd shyamd merged commit 7f6433d into Materials-Consortia:master Jun 12, 2020
@CasperWA CasperWA deleted the fix_331_run-publish-testpypi-on-push branch June 12, 2020 14:12
@CasperWA
Copy link
Member Author

Note: Another solution to this would be to factor out the 'publish_TestPyPI' into a separate CI yml file, which only runs for pushes into 'master'?

This is essentially how we used to do it, and is a way I would prefer, since the current setup introduces some obscurity.

@shyamd as you merged this, could you possibly also make a comment on this (my comment)?

@ml-evs
Copy link
Member

ml-evs commented Jun 12, 2020

Still fails:

HTTPError: 403 Client Error: The user 'ShyamD' isn't allowed to upload to project 'optimade'. See https://test.pypi.org/help/#project-name for more information. for url: https://test.pypi.org/legacy/

Who owns the test pypi package?

@shyamd
Copy link
Contributor

shyamd commented Jun 12, 2020

It properly skips, but doesn't work in the push.

@shyamd
Copy link
Contributor

shyamd commented Jun 12, 2020

@CasperWA , you own the Test PYPI package. I might have overwritten your secret with my key. Can you generate an API token for the Test server and set it in the secrets?

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.

Non-running CI job
3 participants