-
Notifications
You must be signed in to change notification settings - Fork 202
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
Frame dropDepends now drops all depend types #976
Merged
DiegoTavares
merged 3 commits into
AcademySoftwareFoundation:master
from
roulaoregan-spi:fix-cuegui-frame-dropDepends
Jun 20, 2024
Merged
Frame dropDepends now drops all depend types #976
DiegoTavares
merged 3 commits into
AcademySoftwareFoundation:master
from
roulaoregan-spi:fix-cuegui-frame-dropDepends
Jun 20, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
roulaoregan-spi
requested review from
bcipriano,
DiegoTavares,
gregdenton,
IdrisMiles,
jrray,
larsbijl and
smith1511
as code owners
May 17, 2021 16:43
bcipriano
approved these changes
May 18, 2021
@bcipriano - the changes broke the |
bcipriano
reviewed
Jun 5, 2021
bcipriano
requested changes
Jun 5, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment about test line.
MenuActions calling frame.dropDepends only drops FRAME_ON_FRAME dependencies, DependDoJdbc query only checks "pk_frame_depend_er" returns empty list if none found. This does not drop any other types of dependencies. Users create complicated dependency types like FRAME_ON_JOB, FRAME_ON_LAYER etc. that don't always have FRAME_ON_FRAME, so this function would fail to drop the dependencies and caused the frame to be "stuck" in Depend state because all depend types need to be satisfied before the frame is allowed to run. Changed loop to getWhatThisDependsOn() and drop them one by one, allowing the frame to go in "Waiting" and ready to run. (cherry picked from commit 9741efe5f21e524e5c0353115da644698a971488)
The changes to dropDepends broke the MenuActions unittest, because the fn now calls `getWhatThisDependsOn`, which threw a `'Mock' object is not iterable` error. Needed to add depend object for it to pass.
Cuegui MenuActions_tests.py's test_dropDepends unittest no longer applies since the fn no longer calls the pycue's dropDepends with a target parameter. This is a bug fix to dropDepends and now the fn iterates over each dependency and calls "depend.satisfy" instead to remove since the pycue'sdropDepends only applys to "one" depend target, so it will not remove different multiple different dependency types linked to the one frame. Hence, this caused the Cuegui unittest to fail on the Github PR. Opting to put the dropDepends test in pycue (which didn't have one) and remove the test in Cuegui, since Cuegui already has a test coverage for depend.satisfy.
roulaoregan-spi
force-pushed
the
fix-cuegui-frame-dropDepends
branch
from
February 1, 2022 18:36
55761fb
to
0d9e6ed
Compare
DiegoTavares
approved these changes
May 3, 2024
@bcipriano I think you need to approve Roula's change to kick the test pipeline again |
bcipriano
approved these changes
May 9, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summarize your change.
Fixed MenuActions' frame.dropDepends bug which only dropped
FRAME_ON_FRAME
dependencies. The DependDoJdbc query only checkspk_frame_depend_er
and returns empty list if none found, this does not drop any other types of dependencies. Users create complicated dependency types likeFRAME_ON_JOB
,FRAME_ON_LAYER
etc. that don't always have FRAME_ON_FRAME, so this function failed to drop the dependencies and caused the frame to be "stuck" in Depend state because all depend types need to be satisfied before the frame is allowed to run.Changed loop over the different dependencies with
getWhatThisDependsOn()
and drop them one by one, allowing the frame to go in "Waiting" and ready to run.