feat: User Notes on Runs

[camielvs]

Description

Adds a free text field to pipeline runs context panel so users can note down any thoughts or comments.

If the viewing user is not the creator of the run they will not be able to edit the notes.

Also adds an input field to the "Submit Run with Task Arguments" dialog, so that users can enter notes at the time they submit the run.

pipeline-and-run-notes-demo.mov (uploaded via Graphite)

Related Issue and Pull requests

Closes https://github.com/Shopify/oasis-frontend/issues/157

Type of Change

• New feature

Checklist

• I have tested this does not break current pipelines / runs functionality
• I have tested the changes on staging

Screenshots (if applicable)

Test Instructions

• Can add & edit notes on a Pipeline Run
• Can add a note when submitting a run with arguments
• Can READONLY Pipeline Run notes when not the creator of the run
• Run Notes do not copy over when the pipeline is rerun

Additional Comments

[camielvs]

• chore: Centralized Annotation Utilities #1720
• chore: Generalize setRunNameTemplate #1719
• feat: User Notes on Runs #1703 👈 (View in Graphite)
• feat: User Notes on Pipelines #1702
• master

This stack of pull requests is managed by Graphite. Learn more about stacking.

[maxy-shpfy]

Using ref here smells - why do we need it?

[maxy-shpfy]

Ok, moving annotations to "submitPipeline" as an initial payload will solve this problem automatically

[camielvs]

Due to the chaining of queries a useState was not updating correctly and thus passing stale values.

You are correct that using submitPipelineRun to save the annotation alongside the run in one go will fix this, however, submitting annotations via that API does not work as expected.

[maxy-shpfy]

I wish the API /api/pipeline_runs/?include_pipeline_names=true&include_execution_stats=true return some annotations as well so we can display notes in the list. - I believe that could be valuable in some cases.

[maxy-shpfy]

I would change that to send annotations immediately on Run creation, and not AFTER it was created - it will simplify the logic significantly. See https://github.com/TangleML/tangle-ui/blob/master/src/utils/submitPipeline.ts#L63-L65

[camielvs]

it will simplify the logic significantly

It does and I initially tried this. But unless I'm using or understanding the API incorrectly it does not work. Annotations set via submitPipelineRun do not appear when calling GET /api/pipeline_runs/{id}/annotations/ suggesting that the two sets of annotations are stored differently

[maxy-shpfy]

I see. Could we avoid it by a following logic:

1. On submit use TaskSpec annotations to set initial value of notes (in case of presence). This will simplify the entire submitter.
2. On Run View - display immutable notes from TaskSpec, and if user opts in to update notes AFTER - use annotations API. UI will prioritize Notes from API.

And as soon as backend fixes behavior - FE logic would stay exactly as is, no need to redo/refactor after.

[camielvs]

I'm not sure I like either of these solutions. We end up with annotations stored in different places or multiple times.

/api/pipeline_runs/{id}/annotations/ was specifically created for mutable annotations on runs, it would seem weird to avoid using it. It's annoying that the API doesn't behave as expected, but I'd rather use it in its current state than commit to data structures and flows that are completely different to what's intended.

[maxy-shpfy]

I believe you should send annotations immediately on the Run creation, and not AFTER it was created - it will simplify the logic significantly. See https://github.com/TangleML/tangle-ui/blob/master/src/utils/submitPipeline.ts#L63-L65

[camielvs]

^ I tried this. Unfortunately it seems that sending annotations alongside the POST /api/pipeline_runs/ doesn't work or behave as expected. I tried multiple things with the API and found that only the explicit PUT /api/pipeline_runs/{id}/annotations/{key} route works for setting annotations on an execution.

However, I could have missed something or misunderstand the API, so we can pair on this to dive deeper.

[Ark-kun]

I wish the API /api/pipeline_runs/?include_pipeline_names=true&include_execution_stats=true return some annotations as well so we can display notes in the list.

Yes, we should synchronize these things. TangleML/tangle#83

Unfortunately it seems that sending annotations alongside the POST /api/pipeline_runs/ doesn't work or behave as expected.

You're right. Created an issue to fix this: TangleML/tangle#84

P.S. History notes:
I was adding the editable notes/annotations feature on pipeline runs.
But I've realized that we need to make it possible to do optimized search for them.
So I've created a separate table to hold them.
But the rest of the search is not implemented yet.
I wasn't confident enough whether this new table will be good enough for search, so I've postponed integrating it with the rest of the API.

discussed at release meeting

[camielvs]

Merge activity

• Feb 5, 9:48 PM UTC: A user started a stack merge that includes this pull request via Graphite.
• Feb 5, 9:48 PM UTC: @camielvs merged this pull request with Graphite.