Update harmony-service lib and update docker platform to amd64 #33

flamingbear · 2024-10-06T23:12:24Z

Description

Updates the docker image platform to run on amd64

also bumps lib versions of pillow and rasterio hoping to fix DAS-2259 (spoiler alert, it did not)

Updates Harmony-service-lib to version 2

Jira Issue ID

N/A

Local Test Steps

check out the branch, build and run the tests.

❯ ./bin/build-image && ./bin/build-test && ./bin/run-test

bonus points for running regressions locally in Harmony-In-A-Box

PR Acceptance Checklist

[N/A] Jira ticket acceptance criteria met.
CHANGELOG.md updated to include high level summary of PR changes.
docker/service_version.txt updated if publishing a release.
[N/A] Tests added/updated and passing.
[N/A] Documentation updated (if needed).

flamingbear · 2024-10-08T01:13:22Z

bin/build-image

@@ -7,6 +7,7 @@
 # 2022-01-03: Updated Dockerfile path and added comments for tags.
 # 2023-04-04: Updated for the Harmony Browse Image Generator (HyBIG).
 # 2024-01-22: Updated image name to: ghcr.io/nasa/harmony-browse-image-generator.
+# 2024-10-06: Updated to support amd64.


are these actually valuable or just something we keep doing?

That's a good question. I was wondering something similar when I was updating the HGA build-image script. Basically, we're just recreating the commit history in the shell script, so probably they are not uniquely valuable.

Perhaps the one minor value is where the script was originally copied from. But that said, we're trying to standardise, so that shouldn't really matter.

I'm entirely ambivalent beyond saying if we have these comments, they should be complete, or we should not have them. It would be a bit frustrating to have a log of changes to the file and then subsequent changes not captured in them.

Agree. I'll pull them next time I visit (if I remember)

flamingbear · 2024-10-08T16:30:10Z

I was able to run this against hiab locally (even if it was a bit of a slog) [before I updated harmony_service_lib though, I'm trying to do that now, but it should be fine]

lyonthefrog

Update: Regression tests succeeded locally.

Ran the test successfully on my Intel machine and added a couple questions. I'm currently debugging issues with my local Harmony-in-a-Box so I wasn't able to run the regression tests.

Of course Owen still needs to verify that it works on his ARM64 laptop too, but I can approve from my end.

lyonthefrog · 2024-10-14T18:15:34Z

docs/HyBIG-Example-Usage.ipynb

@@ -57,7 +57,7 @@
   "metadata": {},
   "outputs": [],
   "source": [
-    "from harmony import Collection, Environment, Client, Request\n",
+    "from harmony_service_lib import Collection, Environment, Client, Request\n",


So happy we can do this now. Do we need a ticket to update this for all of our regression notebooks, or just sneak it in as we go along?

You could write a ticket, but I'm just dropping it in where I can because those tickets will never get prioritized and if you want to wait until IP sprint I suppose you could.

The elusive IP sprint. I'll just update when it comes up. If I update the granule used in the Trajectory Subsetter regression test, then that'll be a good time.

Just spotting this thread as I opened up the PR - don't the regression test notebooks use the harmony-py library, rather than the harmony-service-lib-py? (If so, that confirms just how much of a good change this renaming is!)

Well I wouldn't know, but that makes sense they would use harmony-py. This has confused me for the entire time I've been on this project.

.pre-commit-config.yaml

I updated all the libs as part of the DAS-2259 ticket, but didn't test it..Oops This reverts commit 7b9690a.

owenlittlejohns

I was able to build the image locally and the unit tests all passed. I tried running the HyBIG regression tests against my local image, and I'm seeing some interesting issues:

"2024-10-15 23:09:29,180 [ERROR] [harmony-service._download:258] Retrying failed download https://harmony.uat.earthdata.nasa.gov/service-results/harmony-uat-eedtest-data/C1256584478-EEDTEST/ASTGTMV003_N00E022_dem.tif?A-api-request-uuid=f63205c0-aa9c-43ec-beb5-2ecb18d9ed95\r\nTraceback (most recent call last):\r\n  File \"/usr/local/lib/python3.11/site-packages/harmony_service_lib/http.py\", line 231, in _download\r\n    raise Exception(f'Unable to download due to status code: {response.status_code} \\\r\nException: Unable to download due to status code: 400                         and content b''\r\n",

I'm not sure why Harmony is unable to download data from that S3 bucket (in it's own AWS account!). I don't think it's a problem with this PR, but maybe one of the @nasa/harmony-admins folks has an idea?

The only change I'm requesting in this review, though, is the reversion of the import change for harmony-py in the Jupyter notebook. Otherwise, this looks great.

owenlittlejohns · 2024-10-15T22:14:28Z

bin/build-image

@@ -7,6 +7,7 @@
 # 2022-01-03: Updated Dockerfile path and added comments for tags.
 # 2023-04-04: Updated for the Harmony Browse Image Generator (HyBIG).
 # 2024-01-22: Updated image name to: ghcr.io/nasa/harmony-browse-image-generator.
+# 2024-10-06: Updated to support amd64.


That's a good question. I was wondering something similar when I was updating the HGA build-image script. Basically, we're just recreating the commit history in the shell script, so probably they are not uniquely valuable.

Perhaps the one minor value is where the script was originally copied from. But that said, we're trying to standardise, so that shouldn't really matter.

I'm entirely ambivalent beyond saying if we have these comments, they should be complete, or we should not have them. It would be a bit frustrating to have a log of changes to the file and then subsequent changes not captured in them.

owenlittlejohns · 2024-10-15T22:57:35Z

docs/HyBIG-Example-Usage.ipynb

    "# creates an output directory for the downloaded files\n",
    "from pathlib import Path\n",
    "\n",
+    "from harmony_service_lib import Client, Collection, Environment, Request\n",


Isn't this harmony-py? I think this one should stay as from harmony import ....

I'm not even joking how I can't keep them straight. ✅ afc0d89

How did the tests pass then when we ran it? I totally see that Client/Collection/etc are in harmony-py, but how were these classes successfully imported?

owenlittlejohns · 2024-10-15T22:58:33Z

harmony_service/adapter.py

+from harmony_service_lib import BaseHarmonyAdapter
+from harmony_service_lib.message import Source as HarmonySource
+from harmony_service_lib.message_utility import (


This makes me happy! I'm so grateful Danny pushed this change through.

owenlittlejohns · 2024-10-15T23:24:01Z

I tried the failed download URL from above in my browser:

https://harmony.uat.earthdata.nasa.gov/service-results/harmony-uat-eedtest-data/C1256584478-EEDTEST/ASTGTMV003_N00E022_dem.tif

That also failed. Not sure if there's something quirky going on with my machine. 🤷

flamingbear · 2024-10-16T00:25:06Z

@owenlittlejohns Made the changes requested, hopefully tomorrow whatever problem with cloudfront is resolved will let you verify

owenlittlejohns · 2024-10-16T17:26:15Z

Okay - I reran the regression tests notebook and things worked for me there today. Not sure what fun was happening with CloudFront, but thank you to whoever/whatever fixed that. I think we're good to merge.

flamingbear added 2 commits October 6, 2024 17:43

Update lib version and update docker platform to amd64

75eb7cb

update changelog

a36cbf5

flamingbear force-pushed the mhs/update-libs branch from 952b936 to a36cbf5 Compare October 6, 2024 23:44

Bump Notebook for synk

cbaa9a8

flamingbear commented Oct 8, 2024

View reviewed changes

DAS-None: Update service versions

45dbf82

flamingbear marked this pull request as ready for review October 8, 2024 16:30

DAS-None update harmony_service_lib to v2.

2e1d99f

flamingbear requested a review from owenlittlejohns October 8, 2024 17:01

flamingbear added 2 commits October 14, 2024 11:06

DAS-2259: pre-commit autoupdate

9157c95

DAS-2259: Update precommit for sorting

3968086

lyonthefrog approved these changes Oct 14, 2024

View reviewed changes

DAS-2259: Update all depedencies.

7b9690a

flamingbear force-pushed the mhs/update-libs branch from 13ce840 to 7b9690a Compare October 15, 2024 13:32

DAS-2259: Revert "DAS-2259: Update all depedencies."

2cc1867

I updated all the libs as part of the DAS-2259 ticket, but didn't test it..Oops This reverts commit 7b9690a.

flamingbear added hacktoberfest-accepted good first issue Good for newcomers and removed good first issue Good for newcomers labels Oct 15, 2024

flamingbear changed the title ~~Update lib version and update docker platform to amd64~~ Update harmony-service lib and update docker platform to amd64 Oct 15, 2024

flamingbear mentioned this pull request Oct 15, 2024

DAS-2259 fix opera rtc browse images. #34

Merged

5 tasks

DAS-2259: Fix links

7b70b3e

flamingbear mentioned this pull request Oct 15, 2024

[pre-commit.ci] pre-commit autoupdate #32

Closed

owenlittlejohns requested changes Oct 15, 2024

View reviewed changes

DAS-2259: I am so glad this this done.

afc0d89

owenlittlejohns approved these changes Oct 16, 2024

View reviewed changes

flamingbear merged commit 117b878 into main Oct 16, 2024
6 checks passed

flamingbear deleted the mhs/update-libs branch October 16, 2024 17:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update harmony-service lib and update docker platform to amd64 #33

Update harmony-service lib and update docker platform to amd64 #33

flamingbear commented Oct 6, 2024 •

edited

Loading

flamingbear Oct 8, 2024

owenlittlejohns Oct 15, 2024

flamingbear Oct 16, 2024

flamingbear commented Oct 8, 2024 •

edited

Loading

lyonthefrog left a comment •

edited

Loading

lyonthefrog Oct 14, 2024

flamingbear Oct 14, 2024

lyonthefrog Oct 14, 2024

owenlittlejohns Oct 15, 2024

flamingbear Oct 15, 2024

owenlittlejohns left a comment

owenlittlejohns Oct 15, 2024

owenlittlejohns Oct 15, 2024

flamingbear Oct 16, 2024

lyonthefrog Oct 16, 2024

owenlittlejohns Oct 15, 2024

owenlittlejohns commented Oct 15, 2024

flamingbear commented Oct 16, 2024

owenlittlejohns commented Oct 16, 2024

Update harmony-service lib and update docker platform to amd64 #33

Update harmony-service lib and update docker platform to amd64 #33

Conversation

flamingbear commented Oct 6, 2024 • edited Loading

Description

Jira Issue ID

Local Test Steps

PR Acceptance Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flamingbear commented Oct 8, 2024 • edited Loading

lyonthefrog left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

owenlittlejohns left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

owenlittlejohns commented Oct 15, 2024

flamingbear commented Oct 16, 2024

owenlittlejohns commented Oct 16, 2024

flamingbear commented Oct 6, 2024 •

edited

Loading

flamingbear commented Oct 8, 2024 •

edited

Loading

lyonthefrog left a comment •

edited

Loading