-
-
Notifications
You must be signed in to change notification settings - Fork 3
fix: Premature link success message #220
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
Merged
in03
merged 24 commits into
main
from
bugfix/issue-214-Premature-linking-success-message
Sep 5, 2022
Merged
fix: Premature link success message #220
in03
merged 24 commits into
main
from
bugfix/issue-214-Premature-linking-success-message
Sep 5, 2022
Conversation
This file contains hidden or 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
- Removed the "spoon-feed/hand-holding" wizard prompts yuck - Also added new exceptions module with proper exceptions for Resolve module - Mutate original jobs with queuer data instead of tasks (needs more work) - Add proper handling for different kinds of link fails - Add logic to handle project change detection
- link_with_mpi is now ProxyLink class - Removed find_and_link linking function, may resurrect in future if necessary, but linking with mpi should cover all reasonable use cases.
…ttps://github.com/in03/resolve-proxy-encoder into bugfix/issue-214-Premature-linking-success-message
…ttps://github.com/in03/resolve-proxy-encoder into bugfix/issue-214-Premature-linking-success-message
in03
reviewed
Sep 5, 2022
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.
Took this a lot further than the original issue scope...
Fixes #214 and #212.
Link logic improvements:
- remove unused
find_and_link()
method - check Resolve is open before linking
- check Resolve project is same before linking
Other improvements:
- add several custom exceptions definitions for Resolve
- improve importing with
__init__py
-level imports - fixed incorrect syntax for private functions
- use tuples instead of lists for linkable type definitions
Some more adjustments to import statements may be necessary for better readability
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.
Problem
Only one file needed to be linked, yet one file linked and one file failed.
They're all the same one??? I reckon that "linked" text isn't running conditionally. Not sure why, but let's figure it out.
Additional Improvements
It would also be nice if we could get a list of the proxies that failed. But we shouldn't assume that the user would always want that. If it's a lot, that'll be a total console spam... Maybe print log location on screen?
closes #214