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

Allow self-referencing product blocks #112

Merged
merged 8 commits into from
Feb 28, 2022

Conversation

Mark90
Copy link
Contributor

@Mark90 Mark90 commented Feb 24, 2022

Closes #108 by using get_type_hints() which evaluates any ForwardRef types to the actual type

@Mark90 Mark90 changed the title Allow nested product blocks Allow self-referencing product blocks Feb 24, 2022
@codecov-commenter
Copy link

codecov-commenter commented Feb 24, 2022

Codecov Report

Merging #112 (7fd00da) into main (3d64546) will increase coverage by 0.04%.
The diff coverage is 87.50%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #112      +/-   ##
==========================================
+ Coverage   83.02%   83.06%   +0.04%     
==========================================
  Files          88       88              
  Lines        5018     5024       +6     
  Branches      830      832       +2     
==========================================
+ Hits         4166     4173       +7     
+ Misses        705      704       -1     
  Partials      147      147              
Impacted Files Coverage Δ
orchestrator/services/processes.py 90.25% <75.00%> (-1.13%) ⬇️
orchestrator/domain/base.py 84.33% <100.00%> (+0.76%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3d64546...7fd00da. Read the comment docs.

@Mark90 Mark90 force-pushed the 108-self-referencing-product-blocks branch from c3c4d25 to 64840c2 Compare February 24, 2022 12:25
@pboers1988
Copy link
Member

Looks good 👍🏼

@pboers1988 pboers1988 self-requested a review February 24, 2022 15:18
@@ -64,6 +64,14 @@ def get_thread_pool() -> ThreadPoolExecutor:
return _workflow_executor


def shutdown_thread_pool() -> None:
Copy link
Member

Choose a reason for hiding this comment

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

Is this only called in the tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, a testcase I added some time ago to test_processes.py (for the resume-all endpoint) turned out to be flaky, as it didn't wait for async tasks to complete.

Adding shutdown_thread_pool() function is the easiest solution for it - the alternative being to patch the threadpoolexecutor, capture .submit() calls and calling .result() on each return value..

I was thinking the threadpool shutdown could/should be used during FastAPI's shutdown, to prevent disconnecting from the DB before all async tasks have concluded. Which has a very small chance of happening, but who knows. :)

@Mark90 Mark90 added this to the 0.3.8 milestone Feb 28, 2022
@Mark90 Mark90 linked an issue Feb 28, 2022 that may be closed by this pull request
@Mark90 Mark90 changed the title Allow self-referencing product blocks #108 Allow self-referencing product blocks Feb 28, 2022
@Mark90 Mark90 changed the title #108 Allow self-referencing product blocks Allow self-referencing product blocks Feb 28, 2022
@Mark90 Mark90 merged commit e3425f1 into main Feb 28, 2022
@Mark90 Mark90 deleted the 108-self-referencing-product-blocks branch February 28, 2022 12:28
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.

Allow self-referencing product blocks
4 participants