Skip to content

fix: typed da classing#1046

Merged
samsja merged 6 commits intofeat-rewrite-v2from
fix-1041-typed-da-classing
Jan 30, 2023
Merged

fix: typed da classing#1046
samsja merged 6 commits intofeat-rewrite-v2from
fix-1041-typed-da-classing

Conversation

@Jackmin801
Copy link
Copy Markdown
Contributor

Goals:

@JohannesMessner
Copy link
Copy Markdown
Member

So if I understand correctly, this basically caches the parametrized classes so that they will not be recreated when it is not necessaray, right? Are there any scenarios where we could end up with two class "instances" with two different caches, such that the original problem could still occur? Maybe through multi-threading/processing? Not sure myself...

@Jackmin801 Jackmin801 force-pushed the fix-1041-typed-da-classing branch from 3e7ea81 to 01e3232 Compare January 24, 2023 09:56
@Jackmin801
Copy link
Copy Markdown
Contributor Author

I dont think worker processes can return a class that was defined in the worker process anyways. So even if different instances existed, it would be illegal for them to interact.

Copy link
Copy Markdown
Member

@samsja samsja left a comment

Choose a reason for hiding this comment

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

neat

Copy link
Copy Markdown
Member

@samsja samsja left a comment

Choose a reason for hiding this comment

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

oups I forgot that I ask for a change

@Jackmin801 Jackmin801 force-pushed the fix-1041-typed-da-classing branch from 9c39fe4 to 01e3232 Compare January 25, 2023 11:13
@github-actions
Copy link
Copy Markdown

📝 Docs are deployed on https://ft-fix-1041-typed-da-classing--jina-docs.netlify.app 🎉

@Jackmin801 Jackmin801 requested a review from samsja January 30, 2023 06:39
@samsja samsja merged commit 3442941 into feat-rewrite-v2 Jan 30, 2023
@samsja samsja deleted the fix-1041-typed-da-classing branch January 30, 2023 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants