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

fix/server blatting #685

Merged
merged 3 commits into from
Sep 6, 2023
Merged

Conversation

stevejpurves
Copy link
Member

@stevejpurves stevejpurves commented Sep 5, 2023

The ThebeServerProvider could replace active servers during navigaion easily, these changes protect against that by checking to see if the server is already "ready" and has a user server url.

This complements well the existing disconnect functionality.

@stevejpurves stevejpurves added bug thebe-react Applies to the thebe-react package labels Sep 5, 2023
@stevejpurves
Copy link
Member Author

A better check on the user server should include pinging the /api/status endpoint. This would be asynchronous so would mean changing the main side effect in the provider to be an explicit action via the connect function, this seems more sensible than the current effect cascade - needs some testing out to understand the impact on consuming applications.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 5, 2023

PR Preview Action v1.4.4
Preview removed because the pull request was closed.
2023-09-06 13:22 UTC

@stevejpurves stevejpurves merged commit 4831168 into fix/tolerant-providers Sep 6, 2023
3 of 4 checks passed
@stevejpurves stevejpurves deleted the fix/server-blatting branch September 6, 2023 13:17
stevejpurves added a commit that referenced this pull request Sep 6, 2023
* 🔧 use correct binder repo url
* 🫴🏼 hooks no longer throw on undefined contexts
* 📕 changeset
* allow custom repoproviderspecs to be added to react server provider
* 🥁don't provide np-ops
* 🙅🏽 throw exception for malformed RepoProviderSpec
* fix/server blatting (#685)
* 📡 add check call for user server
* 👮🏻 prevent reconnect for existing server
* 📕 changeset
stevejpurves added a commit that referenced this pull request Sep 6, 2023
* add binder urls
* server holds url set for binder
* user server url has meaning outside of binder
* fix/tolerant providers (#683)
* 🔧 use correct binder repo url
* 🫴🏼 hooks no longer throw on undefined contexts
* 📕 changeset
* allow custom repoproviderspecs to be added to react server provider
* 🥁don't provide np-ops
* 🙅🏽 throw exception for malformed RepoProviderSpec
* fix/server blatting (#685)
* 📡 add check call for user server
* 👮🏻 prevent reconnect for existing server
* 📕 changeset
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug thebe-react Applies to the thebe-react package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant