Skip to content

Conversation

@bouweandela
Copy link
Member

@bouweandela bouweandela commented Nov 30, 2025

Description

Fix a formatting issue introduced in #2911.


Before you get started

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:

@bouweandela bouweandela added the enhancement New feature or request label Nov 30, 2025
@bouweandela bouweandela marked this pull request as ready for review December 1, 2025 09:40
@bouweandela
Copy link
Member Author

The tests are failing because the server with zarr files used for testing is not available. @valeriupredoi Any idea what's going on there? Could we make the tests more robust to an offline server?

@valeriupredoi
Copy link
Contributor

The tests are failing because the server with zarr files used for testing is not available. @valeriupredoi Any idea what's going on there? Could we make the tests more robust to an offline server?

temporarily unavailable - https://uor-aces-o.s3-ext.jc.rl.ac.uk/esmvaltool-zarr/pr_Amon_CNRM-ESM2-1_02Kpd-11_r1i1p2f2_gr_200601-220112.zarr3/zarr.json works well now - we could make them server-proof, but that resource is pretty much like an ESGF node, very rarely goes offline, problem is that, unlike an ESGF node that can be replaced with another node, that's the only resource

@valeriupredoi
Copy link
Contributor

valeriupredoi commented Dec 1, 2025

WTH is going on with the blithering bucket? Hang on, I spoke too soon 🤦‍♂️

EDIT: clear issue with the S3 object store at CEDA, so I pinged them:
"""
hi folks, is the S3 object store gone fishing? mc ls https://uor-aces-o.s3-ext.jc.rl.ac.uk/esmvaltool-zarr is hanging, and our nightly tests show a 503 trying to read any of the Zarr files in there? Note that mc ls-ing the top level does indeed show the various buckets in there, but accessing any bucket just hangs. Many thanks
"""
@bouweandela since no other tests fail here, and the Zarr store tests did indeed pass on Friday, I say go for the merge 🍺

@bouweandela
Copy link
Member Author

we could make them server-proof

It would be great if you could have a go, this isn't the first time tests are failing because it's offline.

@valeriupredoi
Copy link
Contributor

we could make them server-proof

It would be great if you could have a go, this isn't the first time tests are failing because it's offline.

shall do!

@codecov
Copy link

codecov bot commented Dec 1, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.57%. Comparing base (b8c10a8) to head (778c2f6).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2914   +/-   ##
=======================================
  Coverage   95.57%   95.57%           
=======================================
  Files         266      266           
  Lines       15520    15520           
=======================================
  Hits        14834    14834           
  Misses        686      686           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@valeriupredoi
Copy link
Contributor

should be fixed now, I got in touch with CEDA.

The thing about that S3 bucket is, we could pop a pytest.mark.skipif with some conditional (whuich is not quite easy to figure out), but actually - I kinda need those tests to fail so I can find out there is something wrong with the object store - at the moment, I am pretty much the only one using it, and if I don't get alerted that storage is broken, then it may well stay that way for a fair bit of time; as it was the case now, I alerted CEDA and they fixed the issue. Perhaps they should have such tests themselves, but until then, I'd prefer it fails, what do you think?

@valeriupredoi valeriupredoi merged commit 33c934e into main Dec 2, 2025
4 checks passed
@valeriupredoi valeriupredoi deleted the fix-all-order branch December 2, 2025 09:55
@bouweandela
Copy link
Member Author

I agree that it's a good idea to set up testing for that storage service if the people operating it do not monitor it, but those should be separate tests outside of the ESMValCore repository, because now it is hampering our development.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants