Adding Armada Job Submission Code #43

Sharpz7 · 2022-09-09T16:54:31Z

Closes #39
Closes #50
Closes #41
Closes #45

kannon92 · 2022-09-26T20:29:43Z

Should we close this?

ClifHouck

In addition to my other review comments, I think we need unit test(s) exercising all the new code/behavior in def submit.

armada_jupyter/__main__.py

ClifHouck

Still needs more testing of new code in submit().

Added note in code

Sharpz7 · 2022-09-30T13:42:28Z

Moving to draft as I am going to add the issues into this one that will be needed for the MVP, as they will require significant testing.

#50
#41

Added Sleep for job-set-id creation time

armada_jupyter/constants.py

armada_jupyter/armada.py

tests/unit/test_submit.py

armada_jupyter/__main__.py

armada_jupyter/armada.py

README.md

Co-authored-by: Kevin Hannon <[email protected]>

ClifHouck

A few more comments.

ClifHouck · 2022-10-14T14:30:08Z

armada_jupyter/__main__.py

+        typer.echo(f"Submitted Job {job_id} to Armada")
+
+        # Sleep to make sure that job-set-id is created
+        time.sleep(3)


Is there a better way to do this? Sleeping is a pretty brittle way to wait for something.

This is how the API was designed - to the best of my knowlegde working with the python client over summer there is no other option

I agree with @Sharpz7. This is a problem we found with the Armada API and their focus on being "eventually consistent".

If we this proves to be brittle, we could add an environment variable for this sleep behavior or remove the logic around checking if pod is ready.

As I write this, we may want to relax this. Since Armada is a queueing solution, you could actually have days/weeks before your job starts running. Right now we assume that it takes 3 seconds for your job to be queued and running but this is an invalid assumption.

I think we should change how this operates. All this should do, IMO, is report whether the job is submitted successfully. Beyond that its up to the user to check on the status of their jobs.

I think I'd want more discussion with GR internally before I make that kind of call.

Just to clarify, this sleep is waiting until the event stream becomes active, not for the job to start running - I think those are independent. Therefore, this sleep time is pretty consistently under 3 seconds?

Maybe polling is acceptable in this case?

I have done what I said above, with three "tries" to connect to the event stream, 1s between each try. Regardless of the job states, the job should generally always be created within this time.

As I write this, we may want to relax this. Since Armada is a queueing solution, you could actually have days/weeks before >your job starts running. Right now we assume that it takes 3 seconds for your job to be queued and running but this is an >invalid assumption.

Based on this I am going to add a config option to toggle whether the user wants to use the event stream to wait or not

armada_jupyter/podspec.py

docs/dev/cancel.py

docs/quickstart.md

armada_jupyter/armada.py

Sharpz7 · 2022-10-26T15:53:49Z

Going to split this PR into a docs PR and a code PR

Added queue wait toggle

suprjinx

LGTM

ClifHouck

LGTM

Added initial changes to get it functional for demo

2e6de91

Sharpz7 marked this pull request as draft September 9, 2022 16:54

Sharpz7 self-assigned this Sep 9, 2022

Sharpz7 marked this pull request as ready for review September 19, 2022 11:27

Sharpz7 added 2 commits September 27, 2022 15:10

Merge https://github.com/G-Research/armada-jupyter into final-additions

5b48ba8

Added fixes to finalise merge from commits

8317479

Sharpz7 changed the title ~~Added initial changes to get it functional for demo~~ Adding Armada Job Submission Code Sep 28, 2022

Sharpz7 requested review from jayofdoom and ClifHouck September 28, 2022 16:07

ClifHouck requested changes Sep 29, 2022

View reviewed changes

armada_jupyter/__main__.py Outdated Show resolved Hide resolved

armada_jupyter/__main__.py Outdated Show resolved Hide resolved

armada_jupyter/__main__.py Outdated Show resolved Hide resolved

armada_jupyter/__main__.py Outdated Show resolved Hide resolved

Added suggested changes

50662da

Sharpz7 requested a review from ClifHouck September 29, 2022 15:29

ClifHouck requested changes Sep 29, 2022

View reviewed changes

Added changes to README,

97786e2

Added note in code

Sharpz7 marked this pull request as draft September 30, 2022 13:41

Sharpz7 added 2 commits October 4, 2022 15:27

Added unit test and code for MVP

4e35eaf

Added remap ingress function from armadaproject#45

7e7f76a

Added Sleep for job-set-id creation time

kannon92 reviewed Oct 5, 2022

View reviewed changes

armada_jupyter/constants.py Outdated Show resolved Hide resolved

kannon92 reviewed Oct 5, 2022

View reviewed changes

armada_jupyter/armada.py Outdated Show resolved Hide resolved

Sharpz7 added 2 commits October 7, 2022 18:59

Final unit tests written

feac253

Changed import order

b266b9f

Sharpz7 marked this pull request as ready for review October 7, 2022 17:02

Removed unused constant

a860b18

Sharpz7 commented Oct 7, 2022

View reviewed changes

tests/unit/test_submit.py Outdated Show resolved Hide resolved

Sharpz7 requested review from ClifHouck and kannon92 October 7, 2022 17:06

kannon92 reviewed Oct 7, 2022

View reviewed changes

armada_jupyter/__main__.py Show resolved Hide resolved

kannon92 reviewed Oct 10, 2022

View reviewed changes

armada_jupyter/armada.py Outdated Show resolved Hide resolved

kannon92 reviewed Oct 11, 2022

View reviewed changes

README.md Outdated Show resolved Hide resolved

kannon92 reviewed Oct 11, 2022

View reviewed changes

README.md Outdated Show resolved Hide resolved

Added change to quickstart

b8cb2cd

kannon92 reviewed Oct 11, 2022

View reviewed changes

README.md Outdated Show resolved Hide resolved

Sharpz7 and others added 3 commits October 14, 2022 17:34

Changed client structure

e5e0ff2

Typo Fix 1

a5bca57

Co-authored-by: Kevin Hannon <[email protected]>

Typo Fix 2

b1c05c8

Co-authored-by: Kevin Hannon <[email protected]>

kannon92 previously approved these changes Oct 14, 2022

View reviewed changes

ClifHouck requested changes Oct 17, 2022

View reviewed changes

Sharpz7 commented Oct 17, 2022

View reviewed changes

docs/quickstart.md Outdated Show resolved Hide resolved

Sharpz7 commented Oct 17, 2022

View reviewed changes

docs/quickstart.md Outdated Show resolved Hide resolved

Added dnsrecords docs and namespace docs

d882907

Sharpz7 dismissed kannon92’s stale review via d882907 October 21, 2022 16:10

Sharpz7 requested review from ClifHouck and kannon92 October 21, 2022 16:10

kannon92 reviewed Oct 21, 2022

View reviewed changes

armada_jupyter/armada.py Outdated Show resolved Hide resolved

Sharpz7 added 2 commits October 31, 2022 00:28

Removed docs to make seperate PR

491d781

Added final changes

6eb9096

Sharpz7 requested a review from kannon92 October 31, 2022 01:02

Sharpz7 added 6 commits October 31, 2022 16:12

Reverted README

6135cf9

Removed functions from README that do not exist

88314fb

Attempting rewrite of testing

9f73969

refactored test names.

c68be9e

Added queue wait toggle

Test import fix

a5dbcb1

variable name refactor

c2b3d25

suprjinx approved these changes Nov 1, 2022

View reviewed changes

kannon92 approved these changes Nov 1, 2022

View reviewed changes

ClifHouck approved these changes Nov 1, 2022

View reviewed changes

Sharpz7 merged commit f801728 into armadaproject:main Nov 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding Armada Job Submission Code #43

Adding Armada Job Submission Code #43

Sharpz7 commented Sep 9, 2022 •

edited

Loading

kannon92 commented Sep 26, 2022

ClifHouck left a comment •

edited

Loading

ClifHouck left a comment

Sharpz7 commented Sep 30, 2022 •

edited

Loading

ClifHouck left a comment

ClifHouck Oct 14, 2022

Sharpz7 Oct 17, 2022

kannon92 Oct 21, 2022 •

edited

Loading

ClifHouck Oct 26, 2022

Sharpz7 Oct 26, 2022

Sharpz7 Oct 26, 2022

Sharpz7 Oct 31, 2022

Sharpz7 Oct 31, 2022

Sharpz7 commented Oct 26, 2022

suprjinx left a comment

ClifHouck left a comment

Adding Armada Job Submission Code #43

Adding Armada Job Submission Code #43

Conversation

Sharpz7 commented Sep 9, 2022 • edited Loading

kannon92 commented Sep 26, 2022

ClifHouck left a comment • edited Loading

Choose a reason for hiding this comment

ClifHouck left a comment

Choose a reason for hiding this comment

Sharpz7 commented Sep 30, 2022 • edited Loading

ClifHouck left a comment

Choose a reason for hiding this comment

ClifHouck Oct 14, 2022

Choose a reason for hiding this comment

Sharpz7 Oct 17, 2022

Choose a reason for hiding this comment

kannon92 Oct 21, 2022 • edited Loading

Choose a reason for hiding this comment

ClifHouck Oct 26, 2022

Choose a reason for hiding this comment

Sharpz7 Oct 26, 2022

Choose a reason for hiding this comment

Sharpz7 Oct 26, 2022

Choose a reason for hiding this comment

Sharpz7 Oct 31, 2022

Choose a reason for hiding this comment

Sharpz7 Oct 31, 2022

Choose a reason for hiding this comment

Sharpz7 commented Oct 26, 2022

suprjinx left a comment

Choose a reason for hiding this comment

ClifHouck left a comment

Choose a reason for hiding this comment

Sharpz7 commented Sep 9, 2022 •

edited

Loading

ClifHouck left a comment •

edited

Loading

Sharpz7 commented Sep 30, 2022 •

edited

Loading

kannon92 Oct 21, 2022 •

edited

Loading