Skip to content

feat: allow retries on task profiling steps#691

Merged
guilhem-barthes merged 10 commits into
mainfrom
feat/allow-retries-on-task-profiling-steps
Aug 3, 2023
Merged

feat: allow retries on task profiling steps#691
guilhem-barthes merged 10 commits into
mainfrom
feat/allow-retries-on-task-profiling-steps

Conversation

@guilhem-barthes
Copy link
Copy Markdown
Contributor

@guilhem-barthes guilhem-barthes commented Jul 13, 2023

Description

(4 first commits are refactoring to pytest)

Fixes FL-496

How has this been tested?

Checklist

  • changelog was updated with notable changes
  • documentation was updated

@linear
Copy link
Copy Markdown

linear Bot commented Jul 13, 2023

FL-496 [BUG] call to task profiling POST endpoint triggers a 500

Context and user need:
on lipome/bergonie
failed task on a 500 server error

logs (full logs in comment): unicity constraint violation on (compute_task_profile_id, step)

possibly in the context of a task being retried following an initial, unrelated incident (OOM killed is probable)

Functional spec:

Technical spec:

Acceptance criteria:

@github-actions github-actions Bot added the api label Jul 13, 2023
@guilhem-barthes guilhem-barthes changed the title Feat/allow retries on task profiling steps feat: allow retries on task profiling steps Jul 13, 2023
@guilhem-barthes guilhem-barthes force-pushed the feat/allow-retries-on-task-profiling-steps branch from c560b16 to 9e9e7a4 Compare July 17, 2023 12:12
@guilhem-barthes
Copy link
Copy Markdown
Contributor Author

/e2e

@Owlfred
Copy link
Copy Markdown

Owlfred commented Jul 17, 2023

End to end tests: ✔️ SUCCESS

Nailed it!

@guilhem-barthes guilhem-barthes marked this pull request as ready for review July 17, 2023 12:41
@guilhem-barthes guilhem-barthes requested a review from a team as a code owner July 17, 2023 12:41
Copy link
Copy Markdown
Contributor

@SdgJlbl SdgJlbl left a comment

Choose a reason for hiding this comment

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

Thanks for the PR
I have a few questions to be sure to understand how things are unfolding now :)

Comment thread backend/api/views/task_profiling.py
Comment thread backend/api/views/task_profiling.py
Comment thread backend/api/tests/views/test_views_task_profiling.py
Comment thread backend/substrapp/tasks/tasks_compute_task.py
@guilhem-barthes guilhem-barthes requested review from a team and SdgJlbl July 20, 2023 12:23
@guilhem-barthes guilhem-barthes force-pushed the feat/allow-retries-on-task-profiling-steps branch from 4e074f5 to b64d380 Compare July 26, 2023 09:44
@guilhem-barthes
Copy link
Copy Markdown
Contributor Author

/e2e

@Owlfred
Copy link
Copy Markdown

Owlfred commented Jul 26, 2023

End to end tests: ✔️ SUCCESS

Yay! 🎉

Comment thread backend/api/tests/views/conftest.py
raise e


@retry()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

TIL we have reimplemented our very own retry decorator

Copy link
Copy Markdown
Contributor

@SdgJlbl SdgJlbl left a comment

Choose a reason for hiding this comment

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

Thanks for the PR 🙏

@guilhem-barthes guilhem-barthes force-pushed the feat/allow-retries-on-task-profiling-steps branch from b64d380 to b889a53 Compare July 28, 2023 09:08
@guilhem-barthes
Copy link
Copy Markdown
Contributor Author

/e2e

@Owlfred
Copy link
Copy Markdown

Owlfred commented Jul 28, 2023

End to end tests: ❌ FAILURE

Jobs status:

  • Camelyon: ⏭️
  • Distributed / distributed-sdk:
  • MNIST: ⏭️
  • Standalone / standalone-sdk:

Darn it.

Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com>
Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com>
Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com>
Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com>
Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com>
Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com>
Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com>
Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com>
Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com>
Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com>
@guilhem-barthes guilhem-barthes force-pushed the feat/allow-retries-on-task-profiling-steps branch from b889a53 to 131009b Compare August 2, 2023 15:29
@guilhem-barthes
Copy link
Copy Markdown
Contributor Author

/e2e

@Owlfred
Copy link
Copy Markdown

Owlfred commented Aug 2, 2023

End to end tests: ✔️ SUCCESS

“I love it when a plan comes together.” ― Colonel John “Hannibal” Smith, The A-Team

@guilhem-barthes guilhem-barthes merged commit 30f8c00 into main Aug 3, 2023
@guilhem-barthes guilhem-barthes deleted the feat/allow-retries-on-task-profiling-steps branch August 3, 2023 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants