NASA-Openscapes / earthdata-cloud-cookbook

A tutorial book of workflows for research using NASA EarthData in the Cloud created by the NASA-Openscapes team
https://nasa-openscapes.github.io/earthdata-cloud-cookbook
Other
83 stars 30 forks source link

Start Harmony-py subset example with PO.DAAC data #187

Closed jules32 closed 1 year ago

jules32 commented 1 year ago

@asteiker I'm creating a draft PR to surface your work in progress a bit more

asteiker commented 1 year ago

@jules32 @owenlittlejohns The Harmony team jumped on a quick fix to address the 403 error yesterday. It is now working end to end and I updated subset.qmd accordingly. I do see that we have a separate plotting.qmd so we may be encroaching with the final plotting line in this how-to, but I kept it anyway (perhaps we could leverage this when we get to the plotting how-to and move some of those steps over there). Otherwise I think this can be merged if you think it looks okay.

jules32 commented 1 year ago

Great Amy, and I think you've said a good approach for the plotting.

@owenlittlejohns would you like to review for the harmony-py and merge? I can then double-check the quarto build.

owenlittlejohns commented 1 year ago

The code looks good to me! The request ran, but I'm still getting the 403 when trying to access the data from the 2i2c Hub. Is the Harmony fix a code change that needs to be deployed to production? (For what it's worth the code to get credentials and open in xarray via s3fs also looks solid.)

Also, when merging do you guys have any kind of policy on squashing commits, etc? Or just merging them all as they are in the branch?

jules32 commented 1 year ago

I'll let Amy respond to your code question -

For our reviews/merge, we currently don't have a policy about commits & merging, other than review is required. If you have ideas about squashing we'd love to learn and implement! We'll be formalizing this more this year

asteiker commented 1 year ago

@owenlittlejohns I can reproduce the 403 again as well. It was working yesterday in PROD. I'll work with Harmony to see if something regressed - hopefully we can get it sorted out soon.

owenlittlejohns commented 1 year ago

Okay, @asteiker - I'll hold of re-testing until I get the word on Harmony being sorted in Prod.

@jules32 - with squashing on merge: that's something I've gotten really into over the last year or so, thanks to Matt Savoie. I think it's potentially a great thing for branches like this were there are a lot of WIP-style commits. GitHub is pretty great in giving you a couple of merge options. If you choose "Squash and Merge" it allows you to set the commit message for the final single commit, which defaults (I think) to the PR title. But you can edit that to something super informative. Plus I love the more atomic commit history you get.

jules32 commented 1 year ago

Thanks @owenlittlejohns , I'd love to set up a time for you to teach your favorite collaborative github/review tips, maybe in the Fall with Mentors onboarding? We can help design with you :)

jules32 commented 1 year ago

Great! Are you both all set to merge?

asteiker commented 1 year ago

@jules32 Yes feel free to merge! The Harmony team made some updates to ensure the s3 access changes are permanent now (there was a mishap that caused it to regress initially).

jules32 commented 1 year ago

Great! It's built with the Cookbook now: 🎉 https://nasa-openscapes.github.io/earthdata-cloud-cookbook/how-tos/subset.html