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

S3 support #125

Closed
wants to merge 5 commits into from
Closed

S3 support #125

wants to merge 5 commits into from

Conversation

ocefpaf
Copy link
Member

@ocefpaf ocefpaf commented May 7, 2021

@dopplershift I could not get this to compile locally even with aws-sdk 1.7 as recommended in Unidata/netcdf-c#1981. Do mind taking a look on how upstream do this?

Another question is: will this enable S3 support for all formats or just zarr? Is not how, if possible, can we do that for netcdf too?

Pinging @rsignell-usgs who is interested in this.

@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

Ping @DennisHeimbigner @WardF Is there something we're missing here in the build config? I tried digging into the CI builds in Unidata/netcdf-c, but I didn't see anything obvious that was setting up the S3 testing.

@DennisHeimbigner
Copy link

Another question is: will this enable S3 support for all formats or just zarr? Is not how, if possible, can we do that for netcdf too?

This enables for zarr only. But, the byterange support should work for S3 (as long as it is publically accessible) for netcdf-3 and netcdf-4 files stored as objects in S3.

@DennisHeimbigner
Copy link

We currently do not do Zarr S3 testing on a routine basis since our tests use the NCAR
S3 appliance for testing. Byterange S3 testing is performed, I believe.

@dopplershift
Copy link
Member

@DennisHeimbigner Any pointers on what this build needs in order to properly build against 1.7 of the AWS SDK?

We should also do something about the testing on netCDF-C; this is captured in Unidata/netcdf-c#1994.

@DennisHeimbigner
Copy link

I have had a lot of trouble getting aws-sdk-cpp to compile. At the moment, the only
version I can use is 1.7.301
If you cannot find the code for that version, I can supply it to you, I think.

@ocefpaf
Copy link
Member Author

ocefpaf commented Sep 16, 2021

If you cannot find the code for that version, I can supply it to you, I think.

If it is not in a stable URL the build reproducibility is questionable at best. However, getting that version does not seems to be the issue, building it is. I was not able to make the old version build properly.

@DennisHeimbigner
Copy link

I finally got the latest version of aws-sdk-cpp to build for me on ubuntu 21.
The cmake recipe I used is this:

PREFIX=/usr/local
FLAGS="-DCMAKE_INSTALL_PREFIX=${PREFIX}
       -DCMAKE_INSTALL_LIBDIR=lib \
       -DCMAKE_MODULE_PATH=${PREFIX}/lib/cmake \
       -DCMAKE_POLICY_DEFAULT_CMP0075=NEW \
       -DBUILD_ONLY=s3 \
       -DENABLE_UNITY_BUILD=ON \
       -DENABLE_TESTING=OFF \
       -DCMAKE_BUILD_TYPE=$CFG \
       -DSIMPLE_INSTALL=ON"
cmake -GNinja $FLAGS ..````

@dopplershift
Copy link
Member

@DennisHeimbigner That's great to hear. Have you successfully built netcdf-c with that version?

@DennisHeimbigner
Copy link

Yes, I was able to use it to build netcdf-c with S3 support and successfully run
the status.ucar.edu based tests. I want to create some tests on our
amazon s3 unidata bucket, which is what I have been bugging you about.

@dopplershift
Copy link
Member

@DennisHeimbigner Great. Can you list specifically what version of aws-sdk-cpp. They release often, so "latest release" is quite likely to be incorrect soon.

@DennisHeimbigner
Copy link

Version 1.9

@dopplershift
Copy link
Member

AWS releases a new 1.9.N for each business day. 😆 So which one of those did you use?

@DennisHeimbigner
Copy link

To be precise, 1.9.96.
But in practice it was just the current main.

@djhoese
Copy link
Contributor

djhoese commented Dec 20, 2022

What is the current status of this PR? Is this something that is possible now? I see some of the referenced issues in netcdf-c (with latest aws cpp) are still open.

@DennisHeimbigner
Copy link

At this point, aws-sdk-cpp appears to have reached a reasonably stable point.
So as a rule, I can download it (including submodules), build, install, and use it
in the netcdf-c library.

@djhoese
Copy link
Contributor

djhoese commented Dec 21, 2022

@DennisHeimbigner So you're saying libnetcdf seems to support modern aws-sdk-cpp? So theoretically if this branch was updated with the feedstock main and newest libnetcdf version and newest aws-sdk-cpp it might "just work"?

@dopplershift
Copy link
Member

I would suggest that any new build changes be based on 4.9.1 (I've got the rc building in #154).

@ocefpaf
Copy link
Member Author

ocefpaf commented Dec 21, 2022

I would suggest that any new build changes be based on 4.9.1 (I've got the rc building in #154).

Yep. In fact let's close this one and open a fresh new PR. @djhoese do you want to try that out?

@ocefpaf ocefpaf closed this Dec 21, 2022
@ocefpaf ocefpaf deleted the S3 branch December 21, 2022 18:53
@DennisHeimbigner
Copy link

DennisHeimbigner commented Dec 21, 2022

Yes. It should just work, at least for linux.
There is one caveat I have noticed. There is
an aws-sdk-cpp cmake option to disable AVX support.
Apparently depending on what hardware you have, you
may need to disable the AV support.

@djhoese
Copy link
Contributor

djhoese commented Dec 21, 2022

Given current schedule/TODO/life things I'm not sure I can guarantee working on this. I also know very little about enabling S3 support in NetCDF C apart from what @ocefpaf did in this PR already. If someone gets bored then please open a PR. Otherwise I'll have to wait until I'm avoiding other work I'm supposed to be doing.

@djhoese djhoese mentioned this pull request Dec 22, 2022
5 tasks
@djhoese
Copy link
Contributor

djhoese commented Dec 22, 2022

Local blizzard means kid stayed home from daycare means I work on what I want...

#160

@djhoese djhoese 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