Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

WIP: Airflow: fix undefined poll_interval in Deferrable Operator #2975

Merged
merged 3 commits into from
Sep 18, 2023
Merged

WIP: Airflow: fix undefined poll_interval in Deferrable Operator #2975

merged 3 commits into from
Sep 18, 2023

Conversation

richscott
Copy link
Member

Fix undefined poll_interval attribute in ArmadaJobCompleteTrigger of Deferrable Operator, set it in constructor of the latter class and pass it down to the trigger instantiation. Update tests for it, also fix outdated interval usage in examples, add (the start of) a unit-test specifically for the deferrable operator, make sure we have serialize() and __eq__ implementations for the trigger and operator classes, regenerate updated docs.

@richscott
Airflow: handle poll_interval attr in ArmadaJobCompleteTrigger
2d363d5 
Fix incomplete handling of 'poll_interval' attribute in
ArmadaJobCompleteTrigger, used by the Armada Deferrable Operator for
Airflow.

Signed-off-by: Rich Scott <richscott@sent.com>
@richscott
Airflow - add unit test for armada deferrable operator
1c7dbd9 
Run much of the same tests for the deferrable operator as for the
regular operator, plus test serialization.  Also, update interval
signifier in examples. A full test of the deferrable operator that
verifies the trigger handling is still needed.

Signed-off-by: Rich Scott <richscott@sent.com>
@richscott richscott added do-not-merge Do not merge this PR third-party/airflow Related to Airflow or the Airflow Operator python Pull requests that update Python code labels Sep 16, 2023
@richscott richscott self-assigned this Sep 16, 2023
Sharpz7
Sharpz7 approved these changes Sep 18, 2023
View reviewed changes
Copy link
Contributor

@Sharpz7 Sharpz7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@richscott richscott merged commit 8eb7201 into armadaproject:master Sep 18, 2023
16 checks passed
@richscott richscott deleted the rich/airflow-operator-poll_interval branch September 18, 2023 16:24
svc-gh-ghzonetrans-p pushed a commit that referenced this pull request Sep 19, 2023
@richscott @Mo-Fatah
@d80tb7 @dave-gantenbein @Sharpz7
Wire in min cardinality (#8)
b22b7ca 
* Wire up minimum gang cardinality

* Wire up minimum gang cardinality

* Wire up minimum gang cardinality

* Wire up minimum gang cardinality

* Bump armada airflow operator to version 0.5.4 (#2961)

* Bump armada airflow operator to version 0.5.4

Signed-off-by: Rich Scott <richscott@sent.com>

* Regenerate Airflow Operator Markdown doc.

Signed-off-by: Rich Scott <richscott@sent.com>

* Fix regenerated Airflow doc error.

Signed-off-by: Rich Scott <richscott@sent.com>

* Pin versions of all modules, especially around docs generation.

Signed-off-by: Rich Scott <richscott@sent.com>

* Regenerate Airflow docs using Python 3.10

Signed-off-by: Rich Scott <richscott@sent.com>

---------

Signed-off-by: Rich Scott <richscott@sent.com>

* Infer failed jobs from job context, tidy up

* Infer failed jobs from job context, tidy up

* Magefile: Clean all Makefile refernces (#2957)

* tiny naming change

* clean all make refernces

Signed-off-by: mohamed <mohamedabdelfatah2027@gmail.com>

---------

Signed-off-by: mohamed <mohamedabdelfatah2027@gmail.com>

* Infer failed jobs from job context, tidy up

* Revert to previous unpinned airflow version spec. (#2967)

* Revert to previous unpinned airflow version spec.

Signed-off-by: Rich Scott <richscott@sent.com>

* Increment armada-airflow module version.

Signed-off-by: Rich Scott <richscott@sent.com>

---------

Signed-off-by: Rich Scott <richscott@sent.com>

* Only fail gang jobs when the overall gang min cardinality is set. Fix error handling

* Only fail gang jobs when the overall gang min cardinality is set. Fix error handling

* Only fail gang jobs when the overall gang min cardinality is set. Fix error handling

* Update jobdb with any excess gang jobs that failed

* ArmadaContext.Log Improvements (#2965)

* log error

* context log

* context log

* add cycle id

* typo

* lint

* refactor armadacontext to implement a FieldLogger

---------

Co-authored-by: Chris Martin <chris@cmartinit.co.uk>

* Fix-up existing tests before adding new ones

* Add new tests for minimum gang sizes

* Test that excess failed gang jobs are committed to jobdb

* Run `on.push` only for master (#2968)

* Run On Push only for master

Signed-off-by: mohamed <mohamedabdelfatah2027@gmail.com>

* remove not-workflows

Signed-off-by: mohamed <mohamedabdelfatah2027@gmail.com>

---------

Signed-off-by: mohamed <mohamedabdelfatah2027@gmail.com>

* Add test for failed job pulsar messages

* Tidy tests

* WIP: Airflow: fix undefined poll_interval in Deferrable Operator (#2975)

* Airflow: handle poll_interval attr in ArmadaJobCompleteTrigger

Fix incomplete handling of 'poll_interval' attribute in
ArmadaJobCompleteTrigger, used by the Armada Deferrable Operator for
Airflow.

Signed-off-by: Rich Scott <richscott@sent.com>

* Airflow - add unit test for armada deferrable operator

Run much of the same tests for the deferrable operator as for the
regular operator, plus test serialization.  Also, update interval
signifier in examples. A full test of the deferrable operator that
verifies the trigger handling is still needed.

Signed-off-by: Rich Scott <richscott@sent.com>

---------

Signed-off-by: Rich Scott <richscott@sent.com>

* Release Airflow Operator v0.5.6 (#2979)

Signed-off-by: Rich Scott <richscott@sent.com>

* #2905 - fix indentation (#2971)

Co-authored-by: Mohamed Abdelfatah <39927413+Mo-Fatah@users.noreply.github.com>
Co-authored-by: Adam McArthur <46480158+Sharpz7@users.noreply.github.com>

Signed-off-by: Rich Scott <richscott@sent.com>
Signed-off-by: mohamed <mohamedabdelfatah2027@gmail.com>
Co-authored-by: Rich Scott <rich@gr-oss.io>
Co-authored-by: Mohamed Abdelfatah <39927413+Mo-Fatah@users.noreply.github.com>
Co-authored-by: Chris Martin <council_tax@hotmail.com>
Co-authored-by: Chris Martin <chris@cmartinit.co.uk>
Co-authored-by: Dave Gantenbein <dave@gr-oss.io>
Co-authored-by: Adam McArthur <46480158+Sharpz7@users.noreply.github.com>
severinson added a commit that referenced this pull request Oct 9, 2023
@msumner91 @richscott
@Mo-Fatah @d80tb7 @dave-gantenbein @Sharpz7 @severinson
Gang min cardinality (#2994)
8ad1232 
* Wire in min cardinality (#8)

* Wire up minimum gang cardinality

* Wire up minimum gang cardinality

* Wire up minimum gang cardinality

* Wire up minimum gang cardinality

* Bump armada airflow operator to version 0.5.4 (#2961)

* Bump armada airflow operator to version 0.5.4

Signed-off-by: Rich Scott <richscott@sent.com>

* Regenerate Airflow Operator Markdown doc.

Signed-off-by: Rich Scott <richscott@sent.com>

* Fix regenerated Airflow doc error.

Signed-off-by: Rich Scott <richscott@sent.com>

* Pin versions of all modules, especially around docs generation.

Signed-off-by: Rich Scott <richscott@sent.com>

* Regenerate Airflow docs using Python 3.10

Signed-off-by: Rich Scott <richscott@sent.com>

---------

Signed-off-by: Rich Scott <richscott@sent.com>

* Infer failed jobs from job context, tidy up

* Infer failed jobs from job context, tidy up

* Magefile: Clean all Makefile refernces (#2957)

* tiny naming change

* clean all make refernces

Signed-off-by: mohamed <mohamedabdelfatah2027@gmail.com>

---------

Signed-off-by: mohamed <mohamedabdelfatah2027@gmail.com>

* Infer failed jobs from job context, tidy up

* Revert to previous unpinned airflow version spec. (#2967)

* Revert to previous unpinned airflow version spec.

Signed-off-by: Rich Scott <richscott@sent.com>

* Increment armada-airflow module version.

Signed-off-by: Rich Scott <richscott@sent.com>

---------

Signed-off-by: Rich Scott <richscott@sent.com>

* Only fail gang jobs when the overall gang min cardinality is set. Fix error handling

* Only fail gang jobs when the overall gang min cardinality is set. Fix error handling

* Only fail gang jobs when the overall gang min cardinality is set. Fix error handling

* Update jobdb with any excess gang jobs that failed

* ArmadaContext.Log Improvements (#2965)

* log error

* context log

* context log

* add cycle id

* typo

* lint

* refactor armadacontext to implement a FieldLogger

---------

Co-authored-by: Chris Martin <chris@cmartinit.co.uk>

* Fix-up existing tests before adding new ones

* Add new tests for minimum gang sizes

* Test that excess failed gang jobs are committed to jobdb

* Run `on.push` only for master (#2968)

* Run On Push only for master

Signed-off-by: mohamed <mohamedabdelfatah2027@gmail.com>

* remove not-workflows

Signed-off-by: mohamed <mohamedabdelfatah2027@gmail.com>

---------

Signed-off-by: mohamed <mohamedabdelfatah2027@gmail.com>

* Add test for failed job pulsar messages

* Tidy tests

* WIP: Airflow: fix undefined poll_interval in Deferrable Operator (#2975)

* Airflow: handle poll_interval attr in ArmadaJobCompleteTrigger

Fix incomplete handling of 'poll_interval' attribute in
ArmadaJobCompleteTrigger, used by the Armada Deferrable Operator for
Airflow.

Signed-off-by: Rich Scott <richscott@sent.com>

* Airflow - add unit test for armada deferrable operator

Run much of the same tests for the deferrable operator as for the
regular operator, plus test serialization.  Also, update interval
signifier in examples. A full test of the deferrable operator that
verifies the trigger handling is still needed.

Signed-off-by: Rich Scott <richscott@sent.com>

---------

Signed-off-by: Rich Scott <richscott@sent.com>

* Release Airflow Operator v0.5.6 (#2979)

Signed-off-by: Rich Scott <richscott@sent.com>

* #2905 - fix indentation (#2971)

Co-authored-by: Mohamed Abdelfatah <39927413+Mo-Fatah@users.noreply.github.com>
Co-authored-by: Adam McArthur <46480158+Sharpz7@users.noreply.github.com>

Signed-off-by: Rich Scott <richscott@sent.com>
Signed-off-by: mohamed <mohamedabdelfatah2027@gmail.com>
Co-authored-by: Rich Scott <rich@gr-oss.io>
Co-authored-by: Mohamed Abdelfatah <39927413+Mo-Fatah@users.noreply.github.com>
Co-authored-by: Chris Martin <council_tax@hotmail.com>
Co-authored-by: Chris Martin <chris@cmartinit.co.uk>
Co-authored-by: Dave Gantenbein <dave@gr-oss.io>
Co-authored-by: Adam McArthur <46480158+Sharpz7@users.noreply.github.com>

* Remove unintentional diffs

* Run formatting tools

* Fix formatting

* Re-run proto gen + dotnet gen

* Validate gang min cardinality is <= gang cardinality

* mage proto

* Tidying

---------

Signed-off-by: Rich Scott <richscott@sent.com>
Signed-off-by: mohamed <mohamedabdelfatah2027@gmail.com>
Co-authored-by: Mark Sumner <Mark.Sumner@gresearch.co.uk>
Co-authored-by: Rich Scott <rich@gr-oss.io>
Co-authored-by: Mohamed Abdelfatah <39927413+Mo-Fatah@users.noreply.github.com>
Co-authored-by: Chris Martin <council_tax@hotmail.com>
Co-authored-by: Chris Martin <chris@cmartinit.co.uk>
Co-authored-by: Dave Gantenbein <dave@gr-oss.io>
Co-authored-by: Adam McArthur <46480158+Sharpz7@users.noreply.github.com>
Co-authored-by: Albin Severinson <albin@severinson.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ClifHouck ClifHouck ClifHouck approved these changes

@Sharpz7 Sharpz7 Sharpz7 approved these changes

Assignees

@richscott richscott

Labels
do-not-merge Do not merge this PR python Pull requests that update Python code third-party/airflow Related to Airflow or the Airflow Operator
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@richscott @ClifHouck @Sharpz7