Skip to content
This repository has been archived by the owner on May 13, 2023. It is now read-only.

feat: use single isolate for functions and postgrest #169

Merged
merged 11 commits into from
Jan 13, 2023
Merged

Conversation

dshukertjr
Copy link
Member

What kind of change does this PR introduce?

Instantiates a single instance of YAJsonIsolate and reuses it for Postgrest and functions-dart.

@Vinzent03
Copy link
Contributor

Don't we want the user to be able to provide their own isolate instance? Additionally, I think we should call initialize() on it and like in postgrest dispose it correctly.

@dshukertjr
Copy link
Member Author

@Vinzent03 Hmm, still trying to figure out why test started failing massively once I added initialize() and dispose().

About making it possible to pass custom isolate, would there be any use case where one might want to pass in their custom isolate? I'm a fan of keeping the public API simple, so maybe we can wait until someone requests the feature.

@Vinzent03
Copy link
Contributor

When reverting d804f05 all tests pass for me locally. The isolate.dispose() waits until the init is finished, but doesn't call the init if it has never started.

The use case for keeping the same isolate would be server side for me. Because of the current api you have to construct a new supabase-dart instance every time you want to act as a different user. Being able to keep the same isolate for these short living instances could improve performance.

@dshukertjr
Copy link
Member Author

@Vinzent03 Yeah, I have misread something, and it seems like calling initialize() not the issue. With initialize() being called, the tests do pass for me locally as well, but in Github actions, we get this.

The use case for keeping the same isolate would be server side for me.

Ah, I always forget server side Dart 😂 Yup, sounds good. Let's add an option to pass in an isolate to Supabase client!

* add initialize back in

* run tests against all pr

* add a small delay after each test case

* add option to receive isolate when creating client

* add a longer delay between tests

* remove unnecessary delay

* set web socket channel version
@dshukertjr
Copy link
Member Author

Leaving a note here:

Using web_socket_channel v2.3.0 causes the tests to fail like this. Fixing the version of web_socket_channel to v2.2.0 solves the issue for now.

Copy link
Contributor

@bdlukaa bdlukaa left a comment

Choose a reason for hiding this comment

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

lgtm overall

@dshukertjr dshukertjr requested a review from bdlukaa January 13, 2023 04:35
Copy link
Contributor

@Vinzent03 Vinzent03 left a comment

Choose a reason for hiding this comment

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

LGTM

@dshukertjr dshukertjr merged commit 660d334 into main Jan 13, 2023
@dshukertjr dshukertjr deleted the feat/use-isolate branch January 13, 2023 09:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants