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 overlapping StateDB namespaces #7257

Closed
vidartf opened this issue Sep 23, 2019 · 4 comments · Fixed by #7742
Closed

Allow overlapping StateDB namespaces #7257

vidartf opened this issue Sep 23, 2019 · 4 comments · Fixed by #7742
Assignees
Labels
status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion.
Milestone

Comments

@vidartf
Copy link
Member

vidartf commented Sep 23, 2019

Description

In StateDB, when filtering state based on the namespace, it only checks that the stored value starts with the namespace, not that the namespace matches exactly:

if (val && val.indexOf(query) === 0) {

Reproduce

When using two widget trackers with namespaces myplugin and myplugin-secondary, all widgets from myplugin-secondary will try to be restored for myplugin, since they share a prefix.

Expected behavior

The state for things with different namespaces are segregated.

Proposed solution

Since ids are specified as 'namespace:identifier', StateDB.Connector.list() should split all keys on :, and check for strict equality to the namespace.

@jasongrout
Copy link
Contributor

Since ids are specified as 'namespace:identifier', StateDB.Connector.list() should split all keys on :, and check for strict equality to the namespace.

Is this an explicit requirement? I thought it was basically a convention, but not assumed or enforced.

@jasongrout jasongrout added this to the 1.2 milestone Sep 23, 2019
@vidartf
Copy link
Member Author

vidartf commented Sep 23, 2019

@jasongrout

* identifiers in JupyterLab use as well. While this is not a technical
* requirement for `fetch()`, `remove()`, and `save()`, it *is* necessary for
* using the `list(namespace: string)` method.

@jasongrout
Copy link
Contributor

Thanks. That makes sense. Do you know if this is documented wherever such IDs are needed in our API?

@vidartf
Copy link
Member Author

vidartf commented Sep 23, 2019

I've shared everything I know. Hoping that @afshin can chime in on some of those details since he touched the code last (and therefore by the logic of all children, he now owns it).

@jasongrout jasongrout modified the milestones: 1.2, 2.0 Oct 11, 2019
@lock lock bot added the status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label Feb 4, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Feb 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants