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

[CI Github Actions] Update (outdated) actions versions that produce Node.js warnings #2411

Merged
merged 11 commits into from
May 24, 2024

Conversation

valeriupredoi
Copy link
Contributor

@valeriupredoi valeriupredoi commented May 8, 2024

Description

Three of our actions use outdated versions, therefore one sees a long list of warnings on the action's front page - the problem with oudated actions is that there will be fewer and fewer nodes that actually support them, so that means either run delays or even worse: the RR word (run rejection, don't get any other ideas, like Rolls Royce 😆 )


Checklist

It is the responsibility of the author to make sure the pull request is ready to review. The icons indicate whether the item will be subject to the 🛠 Technical or 🧪 Scientific review.


To help with the number pull requests:

@valeriupredoi valeriupredoi added this to the v2.12.0 milestone May 8, 2024
@valeriupredoi valeriupredoi requested a review from bouweandela May 8, 2024 13:25
Copy link

codecov bot commented May 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.51%. Comparing base (a782af8) to head (ee04170).
Report is 48 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2411   +/-   ##
=======================================
  Coverage   94.51%   94.51%           
=======================================
  Files         246      246           
  Lines       14020    14020           
=======================================
  Hits        13251    13251           
  Misses        769      769           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bouweandela
Copy link
Member

Would it make sense to run the actions to check that they all work?

@valeriupredoi
Copy link
Contributor Author

a good idea, bud! run-tests workflow has all the three updated versions, if that works, then they should all work methinks 🍺

@valeriupredoi
Copy link
Contributor Author

well I'll be a monkey's uncle - OSX is failing miserably - seems there is an issue with masked values of esmf regridding, looking at the esmf build it's different than previously (though the version is the same, just the build hash differs - BTW such an old version!) - I'll investigate tomorrow - good call to run the tests here, bud!

@valeriupredoi
Copy link
Contributor Author

ahahaa! I figured out what the problem is: compare an env that passes vs an env that fails for the same OSX job for the same Python 3.11: the one that passes is from last night's test (nightly), and the failed one is the one from here, so minimal time difference between them - most of the dependencies (if not all!) have the same version, same conda channel (conda-forge) but almost all of them have different build hashes - that's because the new miniconda setup grabs the osx-arm64 builds instead of the bogstandard osx generic builds, see eg:

old: osx-64/netcdf4-1.6.5-nompi_py311hd2be13f_100.conda          6 months and 13 day
new: osx-arm64/netcdf4-1.6.5-nompi_py311ha6bebe6_100.conda       6 months and 13 day

old: osx-64/pandas-2.1.4-py311h1eadf79_0.conda   5 months and 17 hours
new: osx-arm64/pandas-2.1.4-py311h6e08293_0.conda        5 months and 18 hours

that's not entirely wrong since the machines are indeed ARM64:

Operating System
  macOS
  14.4.1
  23E224
Runner Image
  Image: macos-14-arm64
  Version: 20240422.3
  Included Software: https://github.com/actions/runner-images/blob/macos-14-arm64/20240422.3/images/macos/macos-14-arm64-Readme.md
  Image Release: https://github.com/actions/runner-images/releases/tag/macos-14-arm64%2F20240422.3

but the problem is that I am not entirely sure we have actually made our package run on an ARM64 with dedicated ARM64 dependency builds, do you know @bouweandela ? I'd err on the caution side and restrict the OSX deps to osx-64 generic builds, what say you?

@valeriupredoi
Copy link
Contributor Author

this is actually flagged at setup-miniconda GH conda-incubator/setup-miniconda#344 - they recommend using macos-13 and pin to v3- which is blergh

@valeriupredoi
Copy link
Contributor Author

I found out that forcing the Intel architecture as:

  osx:
    runs-on: "macos-latest"
    strategy:
      matrix:
        python-version: ["3.9", "3.10", "3.11"]
        architecture: ["x64"]
      fail-fast: false
    name: OSX Python ${{ matrix.python-version }}
    steps:
      - uses: actions/checkout@v4
        with:
          fetch-depth: 0
      - uses: conda-incubator/setup-miniconda@v3
        with:
          architecture: ${{ matrix.architecture }}
          activate-environment: esmvalcore
          environment-file: environment.yml
          python-version: ${{ matrix.python-version }}
          miniforge-version: "latest"
          miniforge-variant: Mambaforge
          use-mamba: true

works biutifully even with the ARM64 processor and v4, well at least for now, before they diverge completely 🥳

@valeriupredoi
Copy link
Contributor Author

OK @bouweandela everything works again 🍺

@bouweandela
Copy link
Member

bouweandela commented May 9, 2024

All recent Macs have an arm processor, so wouldn't users install the arm version of the packages by default? I think we should test the situation our users are in.

Has the problem with the arm build of esmf already been reported? If arm builds really don't work, we would need to add some extra instructions on how to install to our osx installation instructions.

@valeriupredoi
Copy link
Contributor Author

valeriupredoi commented May 9, 2024

Indeed, but this is a decent patch - it still uses the arm processor snd osx 14, it only switches the packages to be generic osx64, which are still compatible with the arm architecture, when they stop being compatible, it's anyone's guess. The problem with our failed tests with arm64 builds is most probably from netCDF4 or even deeper from hdf5 - I need to investigate, but it's not easy since I don't have access to an M1 Mac (I have a couple ye olde Intel Macs only). For M1 users it's easy - all they have to do is build the env with generic osx64 built dependencies 🍺

@bouweandela
Copy link
Member

For M1 users it's easy - all they have to do is build the env with generoc osx64 built dependencies 🍺

How?

@valeriupredoi
Copy link
Contributor Author

For M1 users it's easy - all they have to do is build the env with generoc osx64 built dependencies 🍺

How?

a man of many words you are, bud 🤣

They use the x86 64bit installer from https://docs.anaconda.com/free/miniconda/ instead of the arm one - this is exactly what the GH action does too via setting the architecture argument:

  ... will download Miniforge.
  Will fetch Mambaforge latest from https://github.com/conda-forge/miniforge/releases/latest/download/Mambaforge-MacOSX-x86_64.sh
  Ensuring Installer...
  Checking for cached Mambaforge@latest...
  Did not find Mambaforge-MacOSX-x86_64.sh latest in cache
  Downloaded Mambaforge-MacOSX-x86_64.sh, ensuring extension .sh
  Caching Mambaforge@latest...
  Cached Mambaforge@latest: /Users/runner/hostedtoolcache/Mambaforge/latest/x86_64!

@valeriupredoi
Copy link
Contributor Author

Cheers @bouweandela 🍺

@valeriupredoi valeriupredoi merged commit 88e53ac into main May 24, 2024
6 checks passed
@valeriupredoi valeriupredoi deleted the update_GA_versions branch May 24, 2024 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants