Try searching multiple ESGF index nodes #1561
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1561 +/- ##
==========================================
+ Coverage 90.76% 90.78% +0.01%
==========================================
Files 197 197
Lines 10572 10594 +22
==========================================
+ Hits 9596 9618 +22
Misses 976 976
Continue to review full report at Codecov.
|
|
The remaining Codacy issues are not easily solved:
|
| 'https://esgf.nci.org.au/esg-search', | ||
| 'https://esgf.nccs.nasa.gov/esg-search', | ||
| 'https://esgdata.gfdl.noaa.gov/esg-search', | ||
| ], |
There was a problem hiding this comment.
cheers @bouweandela - I think it looks good, but I've not tested it, I trust you did, and probably best if you turn on the GA tests and rerun the tests a few times (if you expect a much less frequent failure due to node not being reachable). I do think we should put this list of nodes someplace centrally and just load it from there both for the actual code and for the testing environment, I have seen this list repeated at least three times in different places, and that's prone to introducing issues if we change one node somewhere but not everywhere. What you reckon?
There was a problem hiding this comment.
Unfortunately, I have not been able to test this in real life either, because you need an index node that is timing out for that and I don't think it's that common.
The risk of introducing an error because of the duplication is not that big, the worst thing that can happen is that the documentation goes out of date. The unit test ensures that the default configuration and test code are identical. I agree that this may not be the most elegant unit test, but reading the list from the same source in production and test code is asking for trouble.
There was a problem hiding this comment.
well, not really since that list is a fixed parameter independent of us no? It's like some database that's used both for the functionality and for the functionality test - but, on second thought, that's fine coz if we update the code but not the test, the test will probably keep failing letting us know it needs to be changed 👍
There was a problem hiding this comment.
Unfortunately, I have not been able to test this in real life either, because you need an index node that is timing out for that and I don't think it's that common.
Yeah that's why I was thinking we might pick that up from running a bunch of GA tests - but it's prob too much hassle - we'll have to just wait and see on nightly/PR merge tests I reckon
The missing stubs for |
I don't think it's that easy. When you run prospector locally, things work fine because the stubs are installed as development dependencies. However, this doesn't work on Codacy because Codacy does not install our dependencies. A potential solution could be to run the Codacy command line client in a GitHub action and upload the results, but I've looked at that option for about half an hour and did not immediately see how to get that to work in a maintainable way. |
hahah, that's asking for trouble IMHO. Is Codacy installing the env deps from a container? Coz then that should be configurable. Again, not a biggie for me - am gonna approve, cheers, man! |
I think they're using this to run prospector: https://github.com/codacy/codacy-prospector (i.e. they do not install any dependencies, just |
Description
Closes #1333
Link to documentation: https://esmvaltool--1561.org.readthedocs.build/projects/ESMValCore/en/1561/quickstart/configure.html#search
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: