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

Mock the erddap #252

Merged
merged 45 commits into from Apr 5, 2023
Merged

Mock the erddap #252

merged 45 commits into from Apr 5, 2023

Conversation

gmaze
Copy link
Member

@gmaze gmaze commented Mar 29, 2023

The problem

As mentioned in the last release PR, CI tests suffer from very long execution delays that we suspect to come from remote server connections.

We already created a mocked FTP server in #249 but we have similar issues with all remote server connections.

NB: Dealing with remote servers in CI tests has been a difficulty since the very beginning of argopy (#26 ). Up to this point the strategy was to catch all the possible request errors that were not due to argopy but to the remote server or the connection with a specific fixture

The solution

Here, we extend the mocked server approach to the Ifremer erddap.
In other words, this PR will provide all the necessary fixture and data to mock all Ifremer erddap requests (with fsspec using the argopy httpstore). Hopefully (?) this will dramatically reduce the execution time of CI tests.

@gmaze gmaze added performance internals Internal machinery CI Continuous Integration tools labels Mar 29, 2023
- new log_argopy_callerstack utility for debug
- the httpstore.open_dataset function now enforce the CDF format checking before sending it to xarray
@gmaze
Copy link
Member Author

gmaze commented Mar 31, 2023

Implementation 1

This add the following dependencies for CI environments:

The mocked http server is implemented with aioresponses.
And for this to work we first need to download the expected erddap server raw response and save it on a local file that will be served by the mocked server. This is done out of the CI tests suite (typically on my laptop at this point) using aiofiles.

This approach raises the questions:

  1. how to make sure all expected files used in the CI tests will stay sync with possible evolution of the erddap server ?
  2. how to distribute and access these tests data ? should it be packaged with the software ? should it be on another repo (eg: https://github.com/euroargodev/argopy-data) ? if it stays in here, how do me make sure it's distributed with the pip/conda package ?

RESULTS: 👎
This solution based on aioresponses was not reliable and quite complicated to manage because of the subtleties between HEAD and GET requests. I also could not quite understand how it really worked and did not managed to face some weird stuff like 2 similar requets in a row, the 1st one succeed the 2nd returns a 404 🤬

@gmaze
Copy link
Member Author

gmaze commented Mar 31, 2023

Implementation 2

I dig into the fsspec tests suite and gave a try to their implementation of a simple local server with http.server from here and used by the http store tests.

This is different from the 1st implementation above because here, we don't catch the erddap requests to replace them, but we create a local http server. Therefore, to try this I had to make the erddap data fetcher configurable with regard to the server used.

RESULTS: 👏
This solution seems to work smoothly !
Thanks fsspec for the inspiration

The mocked erddap server is used in:

  • test_fetchers_data_erddap.py
  • test_options.py
  • test_stores.py

gmaze added 10 commits April 3, 2023 10:24
- removed unused _dtype property
Possible option 'disable' for progress bar (used in CI tests)
- use mocked server in httpstore tests
- generalised the mocked erddap to other websites
- replace mockec_erdda_ifremer
@gmaze
Copy link
Member Author

gmaze commented Apr 5, 2023

will merge since this has achieved the PR purpose
will open a new PR to implement the mocked http server approach to all other requests !

@gmaze gmaze merged commit 74e7c82 into master Apr 5, 2023
26 checks passed
@gmaze gmaze deleted the mock-erddap branch April 5, 2023 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration tools internals Internal machinery performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant