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.

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

Add tests for PR #4726 #5200

Merged

Conversation

fg91
Copy link
Member

@fg91 fg91 commented Apr 8, 2024

Add tests for PR #4726. As a separate PR because the PR is getting too long already.

The added tests test the fixes explained in this comment.

@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Apr 8, 2024
@fg91 fg91 marked this pull request as draft April 8, 2024 21:53
@dosubot dosubot bot added the enhancement New feature or request label Apr 8, 2024
Copy link

codecov bot commented Apr 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.09%. Comparing base (d409b3b) to head (9be1964).

Additional details and impacted files
@@                            Coverage Diff                             @@
##           fg91/feat/log-links-show-while-pending    #5200      +/-   ##
==========================================================================
- Coverage                                   61.11%   61.09%   -0.02%     
==========================================================================
  Files                                         793      793              
  Lines                                       51197    51197              
==========================================================================
- Hits                                        31288    31281       -7     
- Misses                                      17032    17039       +7     
  Partials                                     2877     2877              
Flag Coverage Δ
unittests-datacatalog 69.31% <ø> (ø)
unittests-flyteadmin 58.91% <ø> (-0.05%) ⬇️
unittests-flytecopilot 17.79% <ø> (ø)
unittests-flytectl 68.31% <ø> (ø)
unittests-flyteidl 79.30% <ø> (ø)
unittests-flyteplugins 61.94% <ø> (ø)
unittests-flytepropeller 57.32% <ø> (ø)
unittests-flytestdlib 65.73% <ø> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@fg91 fg91 force-pushed the fg91/feat/log-links-show-while-pending branch from d4b547d to dbc36a0 Compare April 17, 2024 19:07
@fg91 fg91 force-pushed the fg91/test/add-tests-pr-4726 branch from 374242b to f343ea7 Compare April 17, 2024 19:29
@fg91 fg91 force-pushed the fg91/feat/log-links-show-while-pending branch from dbc36a0 to 50e76c2 Compare May 23, 2024 20:45
@fg91 fg91 force-pushed the fg91/test/add-tests-pr-4726 branch from 61d2799 to e4bada1 Compare May 24, 2024 17:41
@fg91 fg91 marked this pull request as ready for review May 24, 2024 17:47
@fg91 fg91 requested a review from eapolinario May 24, 2024 17:47
Signed-off-by: Fabio Graetz <[email protected]>
@fg91 fg91 merged commit ecd65a0 into fg91/feat/log-links-show-while-pending Jun 5, 2024
47 of 49 checks passed
@fg91 fg91 deleted the fg91/test/add-tests-pr-4726 branch June 5, 2024 21:35
fg91 added a commit that referenced this pull request Jun 16, 2024
* Add tests to ensure the phase version is bumped in kubeflow plugin if reason changes within the same phase

Signed-off-by: Fabio Graetz <[email protected]>

* Test that ray and dask plugins bump phase version in GetTaskPhase

Signed-off-by: Fabio Graetz <[email protected]>

* Test phase version increase when reason changes for spark plugin

Signed-off-by: Fabio Graetz <[email protected]>

* Fix ray tests after rebase

Signed-off-by: Fabio Graetz <[email protected]>

* Make lint pass

Signed-off-by: Fabio Graetz <[email protected]>

---------

Signed-off-by: Fabio Graetz <[email protected]>
fg91 added a commit that referenced this pull request Jun 16, 2024
* Add tests to ensure the phase version is bumped in kubeflow plugin if reason changes within the same phase

Signed-off-by: Fabio Graetz <[email protected]>

* Test that ray and dask plugins bump phase version in GetTaskPhase

Signed-off-by: Fabio Graetz <[email protected]>

* Test phase version increase when reason changes for spark plugin

Signed-off-by: Fabio Graetz <[email protected]>

* Fix ray tests after rebase

Signed-off-by: Fabio Graetz <[email protected]>

* Make lint pass

Signed-off-by: Fabio Graetz <[email protected]>

---------

Signed-off-by: Fabio Graetz <[email protected]>
eapolinario added a commit that referenced this pull request Jun 17, 2024
* Add ShowWhilePending arg to TaskLog flyteidl message

Signed-off-by: Fabio Graetz <[email protected]>

* Allow showing specific logs already during queued phase

Signed-off-by: Fabio Graetz <[email protected]>

* Use core.PhaseInfoQueuedWithTaskInfo instead of core.PhaseInfoQueued in plugins so log links are available

Signed-off-by: Fabio Graetz <[email protected]>

* Bump phase version in pytorch plugin

Signed-off-by: Fabio Graetz <[email protected]>

* Fix nil containerId in pending phase

Signed-off-by: Fabio Graetz <[email protected]>

* Undo changes from rebase in ray.go

Signed-off-by: Fabio Graetz <[email protected]>

* Regenerate protos

Signed-off-by: Fabio Graetz <[email protected]>

* Fix after rebasing

Signed-off-by: Fabio Graetz <[email protected]>

* Add HideOnceFinished option to TaskLog proto message

Signed-off-by: Fabio Graetz <[email protected]>

* Hide certain logs once finished

Signed-off-by: Fabio Graetz <[email protected]>

* Move log link filtering (by phase) from propeller to admin

Signed-off-by: Fabio Graetz <[email protected]>

* Move bumping of plugin state phase version into function

Signed-off-by: Fabio Graetz <[email protected]>

* Move helper function which bumps phase version to k8s plugin package

Signed-off-by: Fabio Graetz <[email protected]>

* Consistently bump phase version when reason changes in pod, pytorch, tensorflow, and mpi plugins

Signed-off-by: Fabio Graetz <[email protected]>

* Make controlling lifetime of log links work with dask plugin

Signed-off-by: Fabio Graetz <[email protected]>

* Make controlling lifetime of log links work with ray plugin

Signed-off-by: Fabio Graetz <[email protected]>

* Make controlling lifetime of log links work with spark plugin

Signed-off-by: Fabio Graetz <[email protected]>

* Don't return pluginsCore.PhaseInfoUndefined but already known phaseInfo if we fail to update the phase version

Signed-off-by: Fabio Graetz <[email protected]>

* Remove now obsolete logic to check whether dask job is queued

Signed-off-by: Fabio Graetz <[email protected]>

* Adapt docstring explaining why we treat queued and init phase the same while filtering log links

Signed-off-by: Fabio Graetz <[email protected]>

* Make propeller tests pass

Signed-off-by: Fabio Graetz <[email protected]>

* Make pluginmachinery/flytek8s tests pass

Signed-off-by: Fabio Graetz <[email protected]>

* Fix dask, pytorch, tensorflow, and mpi tests

Signed-off-by: Fabio Graetz <[email protected]>

* Make log link filtering by phase work for map tasks

Signed-off-by: Fabio Graetz <[email protected]>

* Add tests for filtering log links when updating task execution

Signed-off-by: Fabio Graetz <[email protected]>

* Show All user logs while queueing phase as before

Signed-off-by: Fabio Graetz <[email protected]>

* Fix spark tests

Signed-off-by: Fabio Graetz <[email protected]>

* Fix after rebase

Signed-off-by: Fabio Graetz <[email protected]>

* Fix flyteidl go.mod

Signed-off-by: Fabio Graetz <[email protected]>

* Fix mpi test

Signed-off-by: Fabio Graetz <[email protected]>

* Add tests for PR #4726 (#5200)

* Add tests to ensure the phase version is bumped in kubeflow plugin if reason changes within the same phase

Signed-off-by: Fabio Graetz <[email protected]>

* Test that ray and dask plugins bump phase version in GetTaskPhase

Signed-off-by: Fabio Graetz <[email protected]>

* Test phase version increase when reason changes for spark plugin

Signed-off-by: Fabio Graetz <[email protected]>

* Fix ray tests after rebase

Signed-off-by: Fabio Graetz <[email protected]>

* Make lint pass

Signed-off-by: Fabio Graetz <[email protected]>

---------

Signed-off-by: Fabio Graetz <[email protected]>

* Update flyteplugins/go/tasks/logs/logging_utils.go

Signed-off-by: Fabio M. Graetz, Ph.D. <[email protected]>
Signed-off-by: Fabio Graetz <[email protected]>

* Update go.mod after flyteidl make generate

Signed-off-by: Fabio Graetz <[email protected]>

* Restrict numpy version in single binary e2e tests

Signed-off-by: Fabio Graetz <[email protected]>

---------

Signed-off-by: Fabio Graetz <[email protected]>
Signed-off-by: Fabio M. Graetz, Ph.D. <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Co-authored-by: Fabio Grätz <[email protected]>
Co-authored-by: Eduardo Apolinario <[email protected]>
robert-ulbrich-mercedes-benz pushed a commit to robert-ulbrich-mercedes-benz/flyte that referenced this pull request Jul 2, 2024
…teorg#4726)

* Add ShowWhilePending arg to TaskLog flyteidl message

Signed-off-by: Fabio Graetz <[email protected]>

* Allow showing specific logs already during queued phase

Signed-off-by: Fabio Graetz <[email protected]>

* Use core.PhaseInfoQueuedWithTaskInfo instead of core.PhaseInfoQueued in plugins so log links are available

Signed-off-by: Fabio Graetz <[email protected]>

* Bump phase version in pytorch plugin

Signed-off-by: Fabio Graetz <[email protected]>

* Fix nil containerId in pending phase

Signed-off-by: Fabio Graetz <[email protected]>

* Undo changes from rebase in ray.go

Signed-off-by: Fabio Graetz <[email protected]>

* Regenerate protos

Signed-off-by: Fabio Graetz <[email protected]>

* Fix after rebasing

Signed-off-by: Fabio Graetz <[email protected]>

* Add HideOnceFinished option to TaskLog proto message

Signed-off-by: Fabio Graetz <[email protected]>

* Hide certain logs once finished

Signed-off-by: Fabio Graetz <[email protected]>

* Move log link filtering (by phase) from propeller to admin

Signed-off-by: Fabio Graetz <[email protected]>

* Move bumping of plugin state phase version into function

Signed-off-by: Fabio Graetz <[email protected]>

* Move helper function which bumps phase version to k8s plugin package

Signed-off-by: Fabio Graetz <[email protected]>

* Consistently bump phase version when reason changes in pod, pytorch, tensorflow, and mpi plugins

Signed-off-by: Fabio Graetz <[email protected]>

* Make controlling lifetime of log links work with dask plugin

Signed-off-by: Fabio Graetz <[email protected]>

* Make controlling lifetime of log links work with ray plugin

Signed-off-by: Fabio Graetz <[email protected]>

* Make controlling lifetime of log links work with spark plugin

Signed-off-by: Fabio Graetz <[email protected]>

* Don't return pluginsCore.PhaseInfoUndefined but already known phaseInfo if we fail to update the phase version

Signed-off-by: Fabio Graetz <[email protected]>

* Remove now obsolete logic to check whether dask job is queued

Signed-off-by: Fabio Graetz <[email protected]>

* Adapt docstring explaining why we treat queued and init phase the same while filtering log links

Signed-off-by: Fabio Graetz <[email protected]>

* Make propeller tests pass

Signed-off-by: Fabio Graetz <[email protected]>

* Make pluginmachinery/flytek8s tests pass

Signed-off-by: Fabio Graetz <[email protected]>

* Fix dask, pytorch, tensorflow, and mpi tests

Signed-off-by: Fabio Graetz <[email protected]>

* Make log link filtering by phase work for map tasks

Signed-off-by: Fabio Graetz <[email protected]>

* Add tests for filtering log links when updating task execution

Signed-off-by: Fabio Graetz <[email protected]>

* Show All user logs while queueing phase as before

Signed-off-by: Fabio Graetz <[email protected]>

* Fix spark tests

Signed-off-by: Fabio Graetz <[email protected]>

* Fix after rebase

Signed-off-by: Fabio Graetz <[email protected]>

* Fix flyteidl go.mod

Signed-off-by: Fabio Graetz <[email protected]>

* Fix mpi test

Signed-off-by: Fabio Graetz <[email protected]>

* Add tests for PR flyteorg#4726 (flyteorg#5200)

* Add tests to ensure the phase version is bumped in kubeflow plugin if reason changes within the same phase

Signed-off-by: Fabio Graetz <[email protected]>

* Test that ray and dask plugins bump phase version in GetTaskPhase

Signed-off-by: Fabio Graetz <[email protected]>

* Test phase version increase when reason changes for spark plugin

Signed-off-by: Fabio Graetz <[email protected]>

* Fix ray tests after rebase

Signed-off-by: Fabio Graetz <[email protected]>

* Make lint pass

Signed-off-by: Fabio Graetz <[email protected]>

---------

Signed-off-by: Fabio Graetz <[email protected]>

* Update flyteplugins/go/tasks/logs/logging_utils.go

Signed-off-by: Fabio M. Graetz, Ph.D. <[email protected]>
Signed-off-by: Fabio Graetz <[email protected]>

* Update go.mod after flyteidl make generate

Signed-off-by: Fabio Graetz <[email protected]>

* Restrict numpy version in single binary e2e tests

Signed-off-by: Fabio Graetz <[email protected]>

---------

Signed-off-by: Fabio Graetz <[email protected]>
Signed-off-by: Fabio M. Graetz, Ph.D. <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Co-authored-by: Fabio Grätz <[email protected]>
Co-authored-by: Eduardo Apolinario <[email protected]>
vlibov pushed a commit to vlibov/flyte that referenced this pull request Aug 16, 2024
…teorg#4726)

* Add ShowWhilePending arg to TaskLog flyteidl message

Signed-off-by: Fabio Graetz <[email protected]>

* Allow showing specific logs already during queued phase

Signed-off-by: Fabio Graetz <[email protected]>

* Use core.PhaseInfoQueuedWithTaskInfo instead of core.PhaseInfoQueued in plugins so log links are available

Signed-off-by: Fabio Graetz <[email protected]>

* Bump phase version in pytorch plugin

Signed-off-by: Fabio Graetz <[email protected]>

* Fix nil containerId in pending phase

Signed-off-by: Fabio Graetz <[email protected]>

* Undo changes from rebase in ray.go

Signed-off-by: Fabio Graetz <[email protected]>

* Regenerate protos

Signed-off-by: Fabio Graetz <[email protected]>

* Fix after rebasing

Signed-off-by: Fabio Graetz <[email protected]>

* Add HideOnceFinished option to TaskLog proto message

Signed-off-by: Fabio Graetz <[email protected]>

* Hide certain logs once finished

Signed-off-by: Fabio Graetz <[email protected]>

* Move log link filtering (by phase) from propeller to admin

Signed-off-by: Fabio Graetz <[email protected]>

* Move bumping of plugin state phase version into function

Signed-off-by: Fabio Graetz <[email protected]>

* Move helper function which bumps phase version to k8s plugin package

Signed-off-by: Fabio Graetz <[email protected]>

* Consistently bump phase version when reason changes in pod, pytorch, tensorflow, and mpi plugins

Signed-off-by: Fabio Graetz <[email protected]>

* Make controlling lifetime of log links work with dask plugin

Signed-off-by: Fabio Graetz <[email protected]>

* Make controlling lifetime of log links work with ray plugin

Signed-off-by: Fabio Graetz <[email protected]>

* Make controlling lifetime of log links work with spark plugin

Signed-off-by: Fabio Graetz <[email protected]>

* Don't return pluginsCore.PhaseInfoUndefined but already known phaseInfo if we fail to update the phase version

Signed-off-by: Fabio Graetz <[email protected]>

* Remove now obsolete logic to check whether dask job is queued

Signed-off-by: Fabio Graetz <[email protected]>

* Adapt docstring explaining why we treat queued and init phase the same while filtering log links

Signed-off-by: Fabio Graetz <[email protected]>

* Make propeller tests pass

Signed-off-by: Fabio Graetz <[email protected]>

* Make pluginmachinery/flytek8s tests pass

Signed-off-by: Fabio Graetz <[email protected]>

* Fix dask, pytorch, tensorflow, and mpi tests

Signed-off-by: Fabio Graetz <[email protected]>

* Make log link filtering by phase work for map tasks

Signed-off-by: Fabio Graetz <[email protected]>

* Add tests for filtering log links when updating task execution

Signed-off-by: Fabio Graetz <[email protected]>

* Show All user logs while queueing phase as before

Signed-off-by: Fabio Graetz <[email protected]>

* Fix spark tests

Signed-off-by: Fabio Graetz <[email protected]>

* Fix after rebase

Signed-off-by: Fabio Graetz <[email protected]>

* Fix flyteidl go.mod

Signed-off-by: Fabio Graetz <[email protected]>

* Fix mpi test

Signed-off-by: Fabio Graetz <[email protected]>

* Add tests for PR flyteorg#4726 (flyteorg#5200)

* Add tests to ensure the phase version is bumped in kubeflow plugin if reason changes within the same phase

Signed-off-by: Fabio Graetz <[email protected]>

* Test that ray and dask plugins bump phase version in GetTaskPhase

Signed-off-by: Fabio Graetz <[email protected]>

* Test phase version increase when reason changes for spark plugin

Signed-off-by: Fabio Graetz <[email protected]>

* Fix ray tests after rebase

Signed-off-by: Fabio Graetz <[email protected]>

* Make lint pass

Signed-off-by: Fabio Graetz <[email protected]>

---------

Signed-off-by: Fabio Graetz <[email protected]>

* Update flyteplugins/go/tasks/logs/logging_utils.go

Signed-off-by: Fabio M. Graetz, Ph.D. <[email protected]>
Signed-off-by: Fabio Graetz <[email protected]>

* Update go.mod after flyteidl make generate

Signed-off-by: Fabio Graetz <[email protected]>

* Restrict numpy version in single binary e2e tests

Signed-off-by: Fabio Graetz <[email protected]>

---------

Signed-off-by: Fabio Graetz <[email protected]>
Signed-off-by: Fabio M. Graetz, Ph.D. <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Co-authored-by: Fabio Grätz <[email protected]>
Co-authored-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Vladyslav Libov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants