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

[Working version for Small 2d and BRT] Small 2D and BRT fixes #409

Closed
wants to merge 21 commits into from

Conversation

annshress
Copy link
Collaborator

@annshress annshress commented Dec 18, 2023

Changes

  • Add notify api running calls as soon as the flow runs start
  • czi flow run uses async/await. Wait for prefect future results before sending it to async calls
  • Remove subflow for small dm conversion flow
  • Revert Completion/Failed to original success/error api calls on notify-api-completion
  • Fix BRT flow: Add dependencies between task runs (replaces wait_for upstream tasks)... Increase cluster size for BRT workflow to be able to handle large number of inputs.
  • cleanup workdir moved to hooks from tasks.
  • Add error for task as messages in callback response using task failure hooks (uses file i/o)

Shortcomings

  • Using file i/o to store errors (this is used to collect error messages to collate in callback response; also, here to store which temporary file locations need to be cleared by on_completion/on_failure hooks) --- Two functions of interest in utils.py are: get_flow_run_tmpdirs and store_exception_hook (for two respective issues).

This PR doesn't introduce any:

  • Binary files
  • Temporary files, auto-generated files
  • Secret keys
  • Local debugging print statements
  • Unwanted comments (e.g: # Gets user from environment for code os.environ['user'] )

This PR contains valid:

  • tests

Copy link

github-actions bot commented Dec 18, 2023

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  em_workflows
  file_path.py 222
  em_workflows/dm_conversion
  flow.py
  em_workflows/lrg_2d_rgb
  flow.py
  em_workflows/utils
  utils.py 72, 81-84, 95-108, 406, 518, 595-598, 607-620
Project Total  

This report was generated by python-coverage-comment-action

em_workflows/brt/flow.py Outdated Show resolved Hide resolved
@annshress annshress force-pushed the fix/small_two_d_fix_and_log branch 9 times, most recently from cab2b16 to 90a617f Compare December 22, 2023 13:22
@annshress annshress marked this pull request as ready for review December 22, 2023 13:32
@annshress annshress closed this Dec 28, 2023
@annshress annshress deleted the fix/small_two_d_fix_and_log branch December 28, 2023 14:25
@annshress annshress restored the fix/small_two_d_fix_and_log branch December 28, 2023 16:39
@annshress annshress reopened this Dec 28, 2023
@annshress annshress marked this pull request as draft December 28, 2023 16:39
@annshress annshress changed the title Fix/small two d fix and log Small 2D and BRT fixes Dec 28, 2023
@annshress
Copy link
Collaborator Author

Closing...

  • file i/o is not the most optimal approach on hooks [ solution: get task run result from hooks directly rather than relying on a hack-y approach ]

@annshress annshress closed this Jan 8, 2024
@annshress annshress changed the title Small 2D and BRT fixes [Working version for BRT] Small 2D and BRT fixes Jan 8, 2024
@annshress annshress changed the title [Working version for BRT] Small 2D and BRT fixes [Working version for Small 2d and BRT] Small 2D and BRT fixes Jan 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants