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

release.yml workflow failing to check new releases #207

Open
maread99 opened this issue Jun 22, 2022 · 20 comments · Fixed by #211, #216, #234 or #373
Open

release.yml workflow failing to check new releases #207

maread99 opened this issue Jun 22, 2022 · 20 comments · Fixed by #211, #216, #234 or #373
Labels
CI Continuous integration TODO Todo list for feature or release

Comments

@maread99
Copy link
Collaborator

maread99 commented Jun 22, 2022

The 4.0 release failed to import as a result of not including the sub-packages in the build (#203). My bad, although it should have been picked up by the release workflow.

The last job of the release workflow is to install and import the newly released version...

- name: Install and test running
run: |
pip install exchange_calendars
python -c 'import exchange_calendars;print(exchange_calendars.__version__)'

However, a look at the workflow logs shows that this job is actually installing and importing the prior version, not the new release. For example the log for the 3.6.3 release shows that the workflow actually installed / imported the prior 3.6.2 release.

In #206 I've explicitly defined the PyPI index that pip should install the package from (although not sure it'll make any difference). Maybe it's simply a matter of PyPI not having had a chance to publish the newly uploaded package?

What I'm struggling with even more is why the prior job which installed from the test PyPI repo also did not fail.

- name: Install from test and test running
run: |
pip install --extra-index-url https://test.pypi.org/simple exchange_calendars
python -c 'import exchange_calendars;print(exchange_calendars.__version__)'
pip uninstall -y exchange_calendars

The log for the 4.0 release workflow seems to show that the version installed and imported was indeed 4.0, although if this were the case then why did it not fail and stop the workflow? (to be clear, it is not possible to install 4.0 from PyPI and import it - rather it fails as described in #203.)

What's also odd is that it appears the Versioneer implementation is not working correctly (#205) in v4. However, the release workflow log shows that exchange_calendars__version__ prints as '4.0' for the package as (supposedly) installed from both the test and the main PyPI repos. By all accounts this should have printed as '0+unknown' given the bug with the Versioneer implementation.

Taking all the above together, it's almost as if although the workflow is downloading and installing a version from PyPI, the version then being imported is a prior locally installed version. Not that I can't see how that could happen?

@gerrymanoim
Copy link
Owner

Hmm - that's very weird. My guess would be that probably pypi hasn't had enough time to process the package we published before we ask to install it?

Perhaps we should either:

  1. Wait a bit after publishing.
  2. Use the github.ref on the actions context to grab the version we want to install and force that constraint in pip install?

@maread99
Copy link
Collaborator Author

#211 introduced to the release workflow a 20s wait between uploading and installing package. Made no difference - the subsequent 4.1 release workflow can be seen to have still tested the release by installing and importing the prior 4.0.2 release.

This leads credence to the idea that it's installing from a previously cached version (which is also supported by the prior behaviour of Versioneer noted above).

  • Prior to next release try either of:
    • Use the github.ref on the actions context to grab the version we want to install and force that constraint in pip install?
    • Prior to the install line add new line pip cache purge?

@maread99 maread99 added TODO Todo list for feature or release CI Continuous integration labels Jun 28, 2022
@maread99 maread99 pinned this issue Jun 28, 2022
@maread99 maread99 reopened this Jul 5, 2022
@maread99
Copy link
Collaborator Author

maread99 commented Jul 5, 2022

Finally fixed in #216. Using github.ref_name to specify the required version seems to have done the job.

@maread99
Copy link
Collaborator Author

This issue was not fixed with #216 after all - the log for the 4.2 release shows that pip couldn't install the uploaded package.

On rsheftel/pandas_market_calendars#208 @Stryder-Git points out that the earlier importing from the test site passes the --extra-index-url option, rather than only looking at the test site with --index-url. Could it be that this causes pip at this point to look for the new version on the main site (to which it hasn't yet been uploaded) and cache that info, then the package is uploaded to the main site and the subsequent call to install the new version fails as pip fails to find the new version in the now outdated cached info?

Going to try changing to --index-url and purging the pip cache. If it's still not working then look to explore @Stryder-Git's further suggestion concerning the repo in the working directory.

@maread99
Copy link
Collaborator Author

maread99 commented Sep 19, 2022

Tried changing to just look at the test repo with --index-url although failed (release 4.2.1) as pip unable to locate all dependencies at test repo. Could have included --no-deps although then it wouldn't be possible to verify that it imports.

Reverted to --extra-url and failed (release 4.2.2), again at the test site stage, as could not find the recently uploaded version. Unsure why as seemed to upload ok and this worked before!

I'll come back to it.

In the meantime, disabling the install and import check (#236) and will check manually.

EDIT: Checked release 4.2.3 manually, installed and imported fine.

@maread99 maread99 pinned this issue Sep 19, 2022
@Stryder-Git
Copy link

@maread99, I also use --no-deps when downloading from testpypi, then I call pip install ., which installs all the dependencies defined in setup.py in the current directory. When I then rename the pandas_market_calendars directory in the local directory, it imports the downloaded package from site-packages and has all the dependencies it needs for running the tests.

In my testing, as part of the workflow, I even inserted a print("downloaded") line at the top of __init__.py before uploading to testpypi. Then I removed the line from the local version, installed from testpypi and ran pytest tests with the -s flag, to see if it prints 'downloaded' on import.

Since doing that, it always worked when I tested it, I hope it helps.

maread99 added a commit that referenced this issue Sep 19, 2022
Attempts fix for install and import checks (#207).
@maread99
Copy link
Collaborator Author

Thank you for that @Stryder-Git. I was struggling yesterday to appreciate the need in your workflow for pip install .. All clear now!

I've had another bash at it (#237) based on your approach. I'll merge it ahead of our next release and take it from there 🤞.

Cheers.

maread99 added a commit that referenced this issue Oct 4, 2022
* Update release workflow

- Attempts fix for install and import checks (#207).
- Revises to wait for newly released version to appear in pip index before
attempting to install.
@maread99
Copy link
Collaborator Author

maread99 commented Oct 4, 2022

Hopefully resolved by #237.

Close if workflow releases next release (after 4.2.3) without a hitch.

@maread99
Copy link
Collaborator Author

4.2.4 released as required (checked manually), although the workflow failed on the subsequent 'Install and import' job. The analogous job to check the upload to the test repo ran fine, and the same workflow has worked with market_prices. Although, on each of those occasions the workflow didn't enter the while loop, which suggests a problem there - seems to have come unstuck on the first iteration or subsequent check of the while condition. @gerrymanoim - would you mind having a look at the loop and letting me know it anything's immediately obvious to you?

I'm clearly missing something. Issue stays open☹️.

@maread99
Copy link
Collaborator Author

maread99 commented Apr 9, 2023

Workflow failed for release 4.2.5 for the same reason as 4.2.4 (although release was ok).

Workflow passed for release 4.2.6 🤷‍♂️ .

@maread99
Copy link
Collaborator Author

maread99 commented May 25, 2023

Workflow passed for release 4.2.7.

Workflow failed for release 4.2.8 on trying to install the newly published version (although release fine, checked manually). Appears that the workflow is failing in the while loop either:

  • on sleep 5s line
  • when trying to augment i with ((i++)). This was previously let i++. If it is this line then both versions fail.
    • If it is this then could try changing it to a for loop, ignore the variable and break when the newly uploaded package appears in the index.

@maread99
Copy link
Collaborator Author

maread99 commented Sep 6, 2023

Workflow failed for release 4.3 in same manner as it failed on 4.2.8. (Again, the actual release was fine - checked manually.)

Going to add an echo to identify if failing on sleeping or the subsequent ((i++)).

@maread99
Copy link
Collaborator Author

I have the same workflow running in the valimp library and was able to identify here that it's failing on the ((i++)). @gerrymanoim, my knowledge of bash is pretty poor, although from what I've seen and what I've tried locally I can't see why this is failing. Any ideas?

@gerrymanoim
Copy link
Owner

I think the issue could be that this is running in sh instead of bash. I think you'll need something like i=$((i+1)). It seems it is a bit unclear exactly what shell you run in (https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsshell), but the i++ syntax works for me locally in bash, but not in sh.

Can also try specifying the shell.

@maread99
Copy link
Collaborator Author

maread99 commented Nov 29, 2023

Thanks @gerrymanoim, looks like that could well be it.

According to the ref you included If bash is not found in the path, this is treated as sh.

Ahead of the next release I'll try explicitly setting the shell for the affected steps. Should just require adding shell: bash above the run | lines for both of the install/import steps.

@maread99
Copy link
Collaborator Author

maread99 commented Jan 2, 2024

I've explicitly defined bash as the shell (#348).

4.5.1 released fine although doesn't appear to have entered the loop so will have to hold off judgement on whether that's done the trick.

@gerrymanoim
Copy link
Owner

🙏

Thanks for grabbing that release!

@maread99
Copy link
Collaborator Author

maread99 commented Jan 31, 2024

4.5.2 did enter the loop and the workflow failed, regardless of the shell being defined as bash. (NB published to PyPI fine, just failed on the subsequent install and import check,)

Failed on the ((i++)) line. Will change to let i++, see if that makes any difference. (EDIT, tagged this change onto #361)

@maread99
Copy link
Collaborator Author

maread99 commented Feb 2, 2024

let i++ doesn't make any difference. Failed on a market-prices release that I'd tried it on.

I'm pretty much out of ideas. Going to add a pre-emptive 5 second sleep between publishing and installing and also remove i from the loop (could end up in an infinite loop, but at the moment it fails anyway, so I can't see that it makes much difference).

@maread99
Copy link
Collaborator Author

Ran fine with the changes for the 4.5.3 release. Didn't enter the loop although with the pre-emptive 5s sleep between publishing and installing I guess there's a reasonable chance it never will.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous integration TODO Todo list for feature or release
Projects
None yet
3 participants