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

Python 3.12 related fixes #7456

Merged
merged 25 commits into from Apr 19, 2024
Merged

Python 3.12 related fixes #7456

merged 25 commits into from Apr 19, 2024

Conversation

poshul
Copy link
Collaborator

@poshul poshul commented Apr 16, 2024

Description

Checklist

  • Make sure that you are listed in the AUTHORS file
  • Add relevant changes and new features to the CHANGELOG file
  • I have commented my code, particularly in hard-to-understand areas
  • New and existing unit tests pass locally with my changes
  • Updated or added python bindings for changed or new classes (Tick if no updates were necessary.)

How can I get additional information on failed tests during CI

Click to expand If your PR is failing you can check out
  • The details of the action statuses at the end of the PR or the "Checks" tab.
  • http://cdash.openms.de/index.php?project=OpenMS and look for your PR. Use the "Show filters" capability on the top right to search for your PR number.
    If you click in the column that lists the failed tests you will get detailed error messages.

Advanced commands (admins / reviewer only)

Click to expand
  • /reformat (experimental) applies the clang-format style changes as additional commit. Note: your branch must have a different name (e.g., yourrepo:feature/XYZ) than the receiving branch (e.g., OpenMS:develop). Otherwise, reformat fails to push.
  • setting the label "NoJenkins" will skip tests for this PR on jenkins (saves resources e.g., on edits that do not affect tests)
  • commenting with rebuild jenkins will retrigger Jenkins-based CI builds

⚠️ Note: Once you opened a PR try to minimize the number of pushes to it as every push will trigger CI (automated builds and test) and is rather heavy on our infrastructure (e.g., if several pushes per day are performed).

@jpfeuffer
Copy link
Contributor

Centos7 is not supported by GitHub Actions anymore. Node was upgraded to 20, which requires a newer glibc.

https://github.blog/changelog/2024-03-07-github-actions-all-actions-will-run-on-node20-instead-of-node16-by-default/

@poshul
Copy link
Collaborator Author

poshul commented Apr 16, 2024

Centos7 is not supported by GitHub Actions anymore. Node was upgraded to 20, which requires a newer glibc.

https://github.blog/changelog/2024-03-07-github-actions-all-actions-will-run-on-node20-instead-of-node16-by-default/

Yeah. I noticed. Any suggestions on the best way to move forward is?

@jpfeuffer
Copy link
Contributor

Only workaround is to update many Linux. Otherwise it will fail mid May.

@poshul
Copy link
Collaborator Author

poshul commented Apr 16, 2024

Only workaround is to update many Linux. Otherwise it will fail mid May.

So this effectively means manylinux2014 is dead for GHA?

.github/workflows/pyopenms-wheels.yml Outdated Show resolved Hide resolved
.github/workflows/python_versions.json Outdated Show resolved Hide resolved
src/pyOpenMS/CMakeLists.txt Outdated Show resolved Hide resolved
@poshul poshul marked this pull request as ready for review April 16, 2024 22:08
@jpfeuffer
Copy link
Contributor

I am pretty sure, yes. If they require node 20 for security (even when downgrading actions versions) then yes, you won't be able to use actions that use nodejs anymore. Which are most of them I guess.

@poshul
Copy link
Collaborator Author

poshul commented Apr 16, 2024

Okay. I can start taking a look at putting together a separate PR for upgrading which manylinux we are using. Tomorrow. It's past my bedtime already

@jpfeuffer
Copy link
Contributor

Hm maybe they keep old action versions running/allowed?
actions/upload-artifact#479

@jpfeuffer
Copy link
Contributor

But the linked PR feels more like a desperate attempt to evade the inevitable

@jpfeuffer
Copy link
Contributor

OpenMS/contrib#142

Just try to update the base container in the Linux branch of the actions here.

@jpfeuffer
Copy link
Contributor

I feel like you should ignore the system compat check in the test, not build step

@timosachsenberg timosachsenberg changed the title Three twelve fixes Python 3.12 related fixes Apr 18, 2024
@poshul poshul requested a review from jpfeuffer April 18, 2024 19:44
@@ -325,6 +333,8 @@ jobs:
test:
needs: [build-win, build-macos, build-lnx]
runs-on: ${{ matrix.os }}
env:
SYSTEM_VERSION_COMPAT: 0 #courtesy of https://github.com/actions/setup-python/issues/469 in lieu of an actual solution.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe also say here that it is a workaround for mac neded for XYZ? above is kind of fine because there it specifies on which os it runs.

Co-authored-by: Timo Sachsenberg <timo.sachsenberg@uni-tuebingen.de>
@poshul poshul merged commit 88f4931 into develop Apr 19, 2024
7 of 8 checks passed
@poshul poshul deleted the three_twelve_fixes branch April 19, 2024 10:07
@ecederstrand
Copy link

@poshul Is there any chance this could reach a version on PyPI soon?

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.

None yet

4 participants