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

Enable Byte-Range request support #107

Merged
merged 3 commits into from
Jul 22, 2020

Conversation

dopplershift
Copy link
Member

netCDF-C has support for accessing remote files using Byte-Range requests, but it's not enabled by default. This PR turns on the support--all it requires is libcurl (already used for OPeNDAP support).

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@dopplershift
Copy link
Member Author

Ping @WardF @DennisHeimbigner any idea why this test would fail?

@WardF
Copy link
Contributor

WardF commented Jul 21, 2020

Taking a look now, not sure offhand. I notice they failures are openmpi; we see failures when using openmpi as it stands, we currently recommend mpich. Last discussion we had about this with members of our HPC community, it was determined to be an issue with openmpi and not an issue we could fix. Let me dig back into that, and also drill down on these failures.

@WardF
Copy link
Contributor

WardF commented Jul 21, 2020

Ok, this is unrelated to openmpi as far as I can tell; ignore the previous comment. Would it be possible, for these tests, to set the CTEST_OUTPUT_ON_FAILURE environmental variable to 1? This would provide more verbose output when the test failed.

@ocefpaf
Copy link
Member

ocefpaf commented Jul 21, 2020

Taking a look now, not sure offhand. I notice they failures are openmpi; we see failures when using openmpi as it stands, we currently recommend mpich. Last discussion we had about this with members of our HPC community, it was determined to be an issue with openmpi and not an issue we could fix. Let me dig back into that, and also drill down on these failures.

All matrix items, no mpi and mpich are failing.

@DennisHeimbigner
Copy link

The travis error I see is because the ./configure build is looking for
aw_cpp_sdk_core library for s3 support. At one point, I was trying to use
PKG_CHECK_MODULES but it appears that I did not understand the proper
operation of this macro. So I gave up on it. I believe this is fixed in the
nczarr-update1.dmh PR, but that has not been merged to master yet.

@dopplershift
Copy link
Member Author

@WardF I tried to turn it on, so hopefully this will give us something useful.

@DennisHeimbigner I'm not sure what you mean about the AWS SDK. I don't see any information about that in the logs. This is for version 4.7.4, which is before any zarr stuff was merged to master as far as I understand.

@DennisHeimbigner
Copy link

I was looking at the travis builds, which failed.

@dopplershift
Copy link
Member Author

dopplershift commented Jul 21, 2020

@DennisHeimbigner The travis logs for this project don't say anything about S3/AWS. Are you getting them switched with the Unidata netcdf-c builds?

@WardF Looks like they fail with:

$SRC_DIR/ncdump/ncdump: https://remotetest.unidata.ucar.edu/thredds/fileServer/testdata/2004050412_eta_211.nc#bytes: https://remotetest.unidata.ucar.edu/thredds/fileServer/testdata/2004050412_eta_211.nc#bytes: NetCDF: Unknown file format

I've been able to reproduce locally (using conda-forge to build).

EDIT: Oh, I see that's what was output before.

@WardF
Copy link
Contributor

WardF commented Jul 21, 2020

To summarize a lot of behind-the-scenes debugging between @dopplershift and I, and using the following command:

$ ./ncdump  https://remotetest.unidata.ucar.edu/thredds/fileServer/testdata/2004050412_eta_211.nc#bytes
  • libcurl 7.69.1 works.
  • libcurl 7.71.1 fails.

Additional info

Other facts of varying usefulness in no particular order.

Conclusions, next steps

At this point, it seems most likely that the issue is either a change in the API for libcurl or is a bug in libcurl.

  • Use the debugger to step through and investigate calls to libcurl to determine the timing and shape of the failure.
  • Compare it against behavior in working version of libcurl.
  • Determine if it's a bug or a change that needs to be accomodated in the netCDF build infrastructure/code.

@dopplershift
Copy link
Member Author

I think we have a fix over at Unidata/netcdf-c#1798. I'll look at adding the patch to the build once it's reviewed.

@ocefpaf
Copy link
Member

ocefpaf commented Jul 22, 2020

🎉

@dopplershift
Copy link
Member Author

Alright, success!

@ocefpaf ocefpaf merged commit 2fb5939 into conda-forge:master Jul 22, 2020
@ocefpaf
Copy link
Member

ocefpaf commented Jul 22, 2020

Thanks @dopplershift. Looking forward to trying this new feature. Hope it does not break the ABI 😄

@dopplershift dopplershift deleted the enable-byterange branch July 22, 2020 20:54
@dopplershift dopplershift mentioned this pull request Jan 4, 2023
5 tasks
@zklaus zklaus mentioned this pull request Jul 6, 2023
5 tasks
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.

5 participants