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

Cannot switch file ID managers with same database #28

Open
davidbrochart opened this issue Oct 26, 2022 · 1 comment · May be fixed by #49
Open

Cannot switch file ID managers with same database #28

davidbrochart opened this issue Oct 26, 2022 · 1 comment · May be fixed by #49
Assignees
Labels
bug Something isn't working
Milestone

Comments

@davidbrochart
Copy link
Contributor

Description

Switching file ID managers from e.g. LocalFileIdManager to ArbitraryFileIdManager doesn't play well with the database:

    HTTPServerRequest(protocol='http', host='127.0.0.1:8888', method='GET', uri='/api/yjs/roomid/Untitled.ipynb?1666790876574', version='HTTP/1.1', remote_ip='127.0.0.1')
    Traceback (most recent call last):
      File "/home/david/mambaforge/envs/jupyterlab/lib/python3.10/site-packages/tornado/web.py", line 1713, in _execute
        result = await result
      File "/home/david/github/davidbrochart/jupyter_server_ydoc/jupyter_server_ydoc/ydoc.py", line 310, in get
        idx = file_id_manager.index(path)
      File "/home/david/github/davidbrochart/jupyter_server_fileid/jupyter_server_fileid/manager.py", line 138, in index
        cursor = self.con.execute("INSERT INTO Files (path) VALUES (?)", (path,))
    sqlite3.IntegrityError: NOT NULL constraint failed: Files.ino

Reproduce

  1. Run JupyterLab in collaborative mode and LocalFileIdManager installed, and open a file.
  2. Run again JupyterLab with an ArbitraryFileIdManager, and open the file.
  3. See the error above.

Expected behavior

Should we use a different database when switching file ID managers? If yes, this issue can be closed.
Otherwise, I'm not sure we should have filesystem-specific attributes in the database. I understand it is needed for local filesystems, but maybe we should give it a more abstract name? In the end, it is some kind of identifier. And other file ID managers might not have this notion, so maybe we should allow it to be NULL?

@davidbrochart davidbrochart added the bug Something isn't working label Oct 26, 2022
@davidbrochart davidbrochart changed the title Cannot switching file ID managers with same database Cannot switch file ID managers with same database Oct 26, 2022
@kevin-bates
Copy link
Member

kevin-bates commented Oct 26, 2022

I think the notion is that each implementation brings its own database schema. I imagine that ArbitraryFileIdManager could become the base class for most implementations (including those based on filesystems that don't want to get down into filesystem details and idiosyncrasies) since it will likely be predicated purely on the event system.

@dlqqq dlqqq added this to the 0.7.0 milestone Nov 7, 2022
@dlqqq dlqqq self-assigned this Nov 7, 2022
@dlqqq dlqqq linked a pull request Nov 9, 2022 that will close this issue
@dlqqq dlqqq modified the milestones: 0.7.0, 0.8.0, 0.9.0 Feb 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants