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: Download cache from the static server #7862

Merged
merged 8 commits into from
Oct 12, 2023
Merged

CI: Download cache from the static server #7862

merged 8 commits into from
Oct 12, 2023

Conversation

seisman
Copy link
Member

@seisman seisman commented Oct 3, 2023

Description of proposed changes

Fixes #

Reminders

  • Make sure that your code follows our style. Use the other functions/files as a basis.
  • Add tests for new features or tests that would have caught the bug that you're fixing.
  • Describe changes to function behavior and arguments in a comment below the function declaration.
  • If adding new functionality, add a detailed description to the documentation and/or an example.

@seisman
Copy link
Member Author

seisman commented Oct 4, 2023

@PaulWessel It seems some datasets are still missing on the static server. For example, earth_age_02m_p is used in the test but it's not available on the static server http:https://static.generic-mapping-tools.org/server/earth/earth_age/.

@PaulWessel
Copy link
Member

I see. Hm, I did run all tests on static and added for instance teh 6m age grid but I must have missed something or had that file in my local static dir. I added the 2m_p now.

@seisman
Copy link
Member Author

seisman commented Oct 5, 2023

There are still many datasets that are not available on static (e.g., @earth_day_01m_p.tif).

I think the easiest way is:

  1. Backup your ~/.gmt/server directory
  2. Delete the existing ~/.gmt/server directory
  3. Run the full tests using the oceanic server
  4. Now, files in the ~/.gmt/server directory are used in the GMT tests and should be copied to the static server. But must be careful with tiled grids (.jp2 on the server vs .nc locally)

@PaulWessel
Copy link
Member

Will follow up - going to Oslo today/evening.

@PaulWessel
Copy link
Member

Mysteries to solve, @seisman: I did the following experiment with an empty .gmt dir:

  1. Enabled set (GMT_DATA_SERVER "static") in my ConfigUserAdvanced.cmake, wiped build dir and completely rebuilt.
  2. Verified that build/test/gmtest issued export GMT_DATA_SERVER=static.
  3. Made sure my terminal had no prior exports via echo $GMT_DATA_SERVER and verified it is empty.
  4. Ran all the tests.
  5. Saved .gmt to .gmt-cmake-static and the log to ~/cmake-static-60.txt as there were 60 failures.

Next, starting with another empty .gmt dir I did this:

  1. Comment out the set (GMT_DATA_SERVER "static") and rebuilt.
  2. In my terminal, did export GMT_DATA_SERVER=static and echo $GMT_DATA_SERVER` reports static.
  3. Ran all the tests.
  4. Saved .gmt to .gmt-environ-static and the log to ~/environ-static-60.txt as there were 50 failures.

Comparing what was downloaded I see

find .gmt-environ-static -type f | wc
197 197 10002
find .gmt-cmake-static -type f | wc
240 240 13166

So 43 more files for the Cmake test. Comparing the logs is harder but note in the Cmake log I have

gmt [ERROR]: Failed to remove /tmp/gmt_hash_server.txt.download! [remove error: No such file or directory]

but that is not in the environ log. There are ~500 more lines in the cmake log. I attach both logs.

environ-static-50.txt
cmake-static-60.txt

My thoughts are these: While in the environment test everything that starts in that terminal find GMT_DATA_SERVER=static, while I am less confident with all the Ctest launches. For instance, in the environ test the gmtest script sets
export GMT_DATA_SERVER=oceania
Surely mixing that and the static environment setting cannot be good.

Finally the term [ERROR] appears in different numbers:

grep '\[ERROR\]' cmake-static-60.txt | wc
     199    1618   14376
grep '\[ERROR\]' environ-static-50.txt | wc
      30     285    2609

I will change gmtest.in so it does not output that line if GMT_DATA_SERVER is found in the environment and report on that.

@seisman
Copy link
Member Author

seisman commented Oct 6, 2023

In the environ-static-50.txt file, I see a warning:

gmt [WARNING]: File /Users/pwessel/.gmt/server/gmt_data_server.txt said it has 416 records but only found 31 - download error???
gmt [WARNING]: File /Users/pwessel/.gmt/server/gmt_data_server.txt should be deleted.  Please try again

It seems the environment test, you're still using the oceanic server. It makes sense to me, because export GMT_DATA_SERVER=static in your terminal should be override by the export GMT_DATA_SERVER=oceanic setting in the gmtest file.

I also see an error in the cmake-static-60.txt file:

gmt [WARNING]: File /Users/pwessel/.gmt/static/gmt_data_server.txt should have 13 fields but only 2 read for record 150 - download error???

I think it's because you were parallelly running the tests, so the gmt_data_server.txt was likely downloaded multiple times at the same time, which may cause a corrupted file.

@PaulWessel
Copy link
Member

Experimenting with environ after removing the export from build/test/gmtest.

@PaulWessel
Copy link
Member

Those errors are now gone, but I stil have a but to fix. WIth static, the cache dir is under static and clearly some code is trying to read the top-level cache (as for oceania). Will work on ths until it works.

Meanwhile, might you know how gmtest.in could be built without that export call if ${GMT_DATA_SERVER} is already set in the environment? Need a Cmake test checking $ENV{???} ?

@seisman
Copy link
Member Author

seisman commented Oct 6, 2023

Meanwhile, might you know how gmtest.in could be built without that export call if ${GMT_DATA_SERVER} is already set in the environment? Need a Cmake test checking $ENV{???} ?

[[ -z "${GMT_DATA_SERVER}" ]] && export GMT_DATA_SERVER=@GMT_DATA_SERVER@

This should work. Only export GMT_DATA_SERVER if it's not defined.

@seisman
Copy link
Member Author

seisman commented Oct 6, 2023

FYI, I've 41 failures for Linux + gs 10.02.0.

@PaulWessel
Copy link
Member

OK, so clearly there are times when two curl jobs fight and we end up with not finding the file when looking. I set N_TEST to 1 and built and then it worked pretty good: 32 failures only. But I had to comment out the line in gmtest if I just wanted to set an environmental variable (cause the default would write oceania. Since it is only the team and maybe some gurus who would care about static, candidate, and test, maybe it is not too much hardship for us to finesse

GMT_DATASERVER=static run tests?

@seisman
Copy link
Member Author

seisman commented Oct 7, 2023

But I had to comment out the line in gmtest if I just wanted to set an environmental variable (cause the default would write oceania. Since it is only the team and maybe some gurus who would care about static, candidate, and test, maybe it is not too much hardship for us to finesse

GMT_DATASERVER=static run tests?

#7862 (comment) Does this work?

@seisman seisman merged commit 8c0d3e2 into master Oct 12, 2023
6 checks passed
@seisman seisman deleted the cache-static branch October 12, 2023 05:18
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

2 participants