Harmony 1894 - add support for labeling jobs #630

indiejames · 2024-09-26T18:26:20Z

Jira Issue ID

HARMONY-1894

Description

Adds a new parameter label that can be provided with Harmony requests (OGC Coverages/EDR) to specify one ore more labels for a job.

Local Test Steps

Check out this branch
Run the migrations or run bin/bootstrap-harmony which will run the migrations for you
Include one or more labels with a Harmony request
For Coverages:

curl -Ln -bj "http://localhost:3000/C1233800302-EEDTEST/ogc-api-coverages/1.0.0/collections/red_var,blue_var,green_var/coverage/rangeset/?subset=lat(20%3A60)&subset=lon(-140%3A-50)&outputCrs=EPSG%3A31975&format=image%2Fpng&maxResults=1&label=Foo,baR"

For EDR (GET):

curl -Ln -bj "http://localhost:3000/ogc-api-edr/1.1.0/collections/C1234208438-POCLOUD/area?maxResults=1&parameter-name=all&coords=POLYGON%20%28%28-164.8%2032.3%2C%20-179.3%2025.2%2C%20-165.5%2018.3%2C%20-164.8%2032.3%29%29&labels=abc,123"

For EDR (POST):

curl --request POST \
        --url http://localhost:3000/ogc-api-edr/1.1.0/collections/C1233800302-EEDTEST/position \
        --header "Authorization: Bearer $MY_UAT_TOKEN" \
        --header 'Content-Type: application/json' \
        --data '{
    "coords": "POINT(-40 10)",
    "granuleId": ["G1233800343-EEDTEST"],
    "parameter-name": [
      "all"
    ],
    "crs": "EPSG:31975",
    "f": "image/png", "label": ["abc😡123","Fu#bar"]
  }'

Check the database tables labels and job_labels and make sure you see rows for the labels you set.
Here are a couple of useful queries (change the labels to whatever you used):

select "j"."jobID" from jobs j JOIN jobs_labels l ON "j"."jobID" = l.job_id JOIN labels ul ON l.label_id = ul.id WHERE ul.value = 'fizz';
select "j"."jobID" from jobs j JOIN jobs_labels l ON "j"."jobID" = l.job_id JOIN labels ul ON l.label_id = ul.id WHERE ul.value = 'abc😡123';

PR Acceptance Checklist

Acceptance criteria met
Tests added/updated (if needed) and passing
Documentation updated (if needed)

to avoid merge issues and add a rethrow of errors that were being captured in job saves

to the jobs table and some other cleanup

description

chris-durbin · 2024-09-26T19:13:19Z

services/harmony/app/models/label.ts

+ * @param labels - the array of strings representing the labels. These will be forced to lower-case.
+ * If this is an empty array then any existing labels for the job will be cleared.
+ */
+export async function setLabelsForJob(


I'm not positive how this is going to be used, but I do not like endpoints that require you to pass in what you think should exist plus any new info.

I'd want to have an endpoint to add one or more labels without changing any existing labels and an endpoint for removing one or more labels.

The AC for this ticket:

* when a user submits a request they can submit one or more labels for the job * the new label parameter is documented with examples

This ticket just allows the user to set labels when making a request. So not updating things. The updates will happen in later tickets. But one of the major use cases for the other tickets is to allow users to add labels from the workflow-ui, in which case I would think the UI should show them the existing labels, which means you would have them and could resend the whole list. But if you want to do it piecemeal for some reason then that can be a different function.

One thing I have tried to avoid is adding a full model for labels, since they are so simple, but if you want CRUD at the label level then maybe we should make a full model.

chris-durbin

I tested things out successfully in a sandbox environment. I ran into one issue with a 500 error returned (I assume because of the 255 character limit) when passing in a wall of text for the label. I think we have two options:

Reject the request with a 400 error and say each label is limited to 255 characters
Accept the request and truncate the label to 255 characters

indiejames · 2024-09-26T20:28:09Z

I tested things out successfully in a sandbox environment. I ran into one issue with a 500 error returned (I assume because of the 255 character limit) when passing in a wall of text for the label. I think we have two options:

Reject the request with a 400 error and say each label is limited to 255 characters

Accept the request and truncate the label to 255 characters

I vote for option 1. I was already thinking that I should add a line in the docs about the 255 character limit.

bin/stop-harmony-and-services

ygliuvt · 2024-09-27T12:45:41Z

db/db.sql

+  `value` varchar(255) not null,
+  `createdAt` datetime not null,
+  `updatedAt` datetime not null,
+  UNIQUE(username, value)


Why don't we make the value itself be the unique key?

What is the use case of username and value?

If we did that then only one person could be associated with the label value. We want to be able to look up the labels for a person so we can populate an autocomplete, so we need to have both value and username here.

When I log in as an admin, I think I should see all labels rather than just mine. There will be so many duplicate label values in the table and that will make cleanup much harder.

We can get the labels for a user through the jobs table and the association with labels.

ygliuvt · 2024-09-27T13:27:56Z

services/harmony/test/models/label.ts

+  describe('update labels for job', async function () {
+    const updatedLabels = ['baz', 'buzz'];
+    it('updates the labels for the job', async function () {
+      await setLabelsForJob(this.trx, this.job.jobID, this.job.username, labels);


nit: We can remove this line if we make the whole test a transaction with three steps and I think the test will be more readable if we do that.

I like for tests to not rely on other tests setting things up in the database. It makes them too fragile.

ygliuvt · 2024-09-27T13:29:38Z

services/harmony/app/models/label.ts

+  if (!labels) return;
+
+  // delete any labels that already exist for the job
+  await transaction('jobs_labels')


what about the labels table?

we don't want to delete those. Labels can be re-used.

add maximum length validation

chris-durbin

Verified functionality for rejecting requests with labels greater than 255 along and verified 255 or less continued to work as expected.

indiejames added 6 commits September 25, 2024 15:00

HARMONY-1894: Add ability to set labels for a job

226827c

HARMONY-1894: Fix broken tests

c8c05b7

HARMONY-1894: Move migration files to later timestamp

96e75dd

to avoid merge issues and add a rethrow of errors that were being captured in job saves

HARMONY-1894: Merge branch 'main' into harmony-1894

c6991c3

HARMONY-1894: Renamed labels tables to better reflect the relationship

0f40f72

to the jobs table and some other cleanup

HARMONY-1894: Small documentation change for label

97401a7

description

indiejames requested review from chris-durbin, vinnyinverso and ygliuvt September 26, 2024 18:27

chris-durbin reviewed Sep 26, 2024

View reviewed changes

ygliuvt reviewed Sep 27, 2024

View reviewed changes

bin/stop-harmony-and-services Outdated Show resolved Hide resolved

ygliuvt reviewed Sep 27, 2024

View reviewed changes

HARMONY-1894: Rework label parameter handling and

0c92c27

add maximum length validation

indiejames requested review from ygliuvt and chris-durbin September 27, 2024 16:14

HARMONY-1894: Fix error message for exceeding label max characters

180267f

chris-durbin approved these changes Sep 27, 2024

View reviewed changes

ygliuvt approved these changes Sep 27, 2024

View reviewed changes

HARMONY-1894: Fix formatting in label docs

a83360d

indiejames merged commit c0250ec into main Sep 27, 2024
4 of 6 checks passed

indiejames deleted the harmony-1894 branch September 27, 2024 18:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Harmony 1894 - add support for labeling jobs #630

Harmony 1894 - add support for labeling jobs #630

indiejames commented Sep 26, 2024 •

edited

Loading

chris-durbin Sep 26, 2024

indiejames Sep 26, 2024 •

edited

Loading

indiejames Sep 26, 2024 •

edited

Loading

chris-durbin left a comment

indiejames commented Sep 26, 2024

ygliuvt Sep 27, 2024

ygliuvt Sep 27, 2024

indiejames Sep 27, 2024

ygliuvt Sep 27, 2024 •

edited

Loading

ygliuvt Sep 27, 2024

ygliuvt Sep 27, 2024

indiejames Sep 27, 2024

ygliuvt Sep 27, 2024

indiejames Sep 27, 2024

chris-durbin left a comment

Harmony 1894 - add support for labeling jobs #630

Harmony 1894 - add support for labeling jobs #630

Conversation

indiejames commented Sep 26, 2024 • edited Loading

Jira Issue ID

Description

Local Test Steps

PR Acceptance Checklist

Choose a reason for hiding this comment

indiejames Sep 26, 2024 • edited Loading

Choose a reason for hiding this comment

indiejames Sep 26, 2024 • edited Loading

Choose a reason for hiding this comment

chris-durbin left a comment

Choose a reason for hiding this comment

indiejames commented Sep 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ygliuvt Sep 27, 2024 • 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

chris-durbin left a comment

Choose a reason for hiding this comment

indiejames commented Sep 26, 2024 •

edited

Loading

indiejames Sep 26, 2024 •

edited

Loading

indiejames Sep 26, 2024 •

edited

Loading

ygliuvt Sep 27, 2024 •

edited

Loading