Skip to content

Use IDrive instead of file browser#29

Open
juntyr wants to merge 5 commits intojupyterlite:mainfrom
juntyr:idrive
Open

Use IDrive instead of file browser#29
juntyr wants to merge 5 commits intojupyterlite:mainfrom
juntyr:idrive

Conversation

@juntyr
Copy link
Copy Markdown
Contributor

@juntyr juntyr commented Apr 10, 2026

Maybe this helps with #28?

@github-actions
Copy link
Copy Markdown

Binder 👈 Launch a Binder on branch juntyr/litegitpuller/idrive

@juntyr
Copy link
Copy Markdown
Contributor Author

juntyr commented Apr 10, 2026

@jtpio What do you think (this is the first step towards #31)?

@jtpio jtpio requested a review from brichet April 10, 2026 12:32
@jtpio
Copy link
Copy Markdown
Member

jtpio commented Apr 10, 2026

Thanks @juntyr for looking into this.

cc @brichet who commented on the original issue and may want to have a look.

Looks like some of the CI failures are not related.

@brichet
Copy link
Copy Markdown
Collaborator

brichet commented Apr 13, 2026

Thanks @juntyr.
#32 should fix the CI failure not related to this PR.

@juntyr
Copy link
Copy Markdown
Contributor Author

juntyr commented Apr 13, 2026

@brichet How do I update the lockfile?

@brichet
Copy link
Copy Markdown
Collaborator

brichet commented Apr 13, 2026

Running jlpm install should update the lock file locally, it just need to be committed.

@brichet
Copy link
Copy Markdown
Collaborator

brichet commented Apr 13, 2026

Also, for consistency (and , if may worth bumping all the @jupyterlab dependencies all together (@jupyterlab/application and @jupyterlab/coreutils).

And running jlpm dlx yarn-berry-deduplicate -s fewerHighest may help avoiding duplicate dependencies in different version (it's often helpful for @jupyterlab and @lumino dependencies).

@brichet brichet added enhancement New feature or request bug Something isn't working and removed enhancement New feature or request labels Apr 13, 2026
Comment thread src/gitpuller.ts
Comment on lines +151 to +165
const newFile = await this._drive.newUntitled({
type: (ext === '.ipynb' ? 'notebook' : 'file') as Contents.ContentType,
path: PathExt.dirname(filePath),
ext: ext
});
await this._drive.save(newFile.path, {
content:
newFile.format === 'json'
? JSON.parse(await blob.text())
: newFile.format === 'text'
? await blob.text()
: await blobToBase64(blob),
size: blob.size
});
await this._drive.rename(newFile.path, filePath);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't know how the upload handles e.g. mimetypes so I tried to leave as much of the guessing up to the newUntitled method by giving it the file extension. However, this approach is likely less efficient, so I'd be happy for insights from @jtpio and @brichet

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I wonder if we need to use a newUntilted, can't we directly use the save method ?

And maybe we can guess if it is a text or bytes mimetype using something like the following (suggestion from chatGPT, not sure if it misses some case):

function isTextMimeType(mime: string): boolean {
  const textTypes = [
    "application/json",
    "application/xml",
    "application/javascript",
    "application/x-www-form-urlencoded",
    "image/svg+xml"
  ];

  return (
    mime.startsWith("text/") ||
    textTypes.includes(mime) ||
    mime.endsWith("+json") || // e.g. application/ld+json
    mime.endsWith("+xml")
  );
}

I tested successfully the following implementation, with the example from RTD (which contains only notebook, image and text files).

    const isText = isTextMimeType(type);
    const content = isText ? await blob.text() : await blobToBase64(blob);
    await this._drive.save(filePath, {
      content: content,
      type: 'file',
      mimetype: type,
      format: isText ? 'text' : 'base64'
    });

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is the logic that the JupyterLite browser storage drive uses: https://github.com/jupyterlite/jupyterlite/blob/b0f84a44894e3edc554bb0e03f193c44d05f20ff/packages/services/src/contents/drive.ts#L398-L431

I fear that we'd have to duplicate this logic exactly, and the current approach circumvents this code duplication

@juntyr
Copy link
Copy Markdown
Contributor Author

juntyr commented Apr 13, 2026

The RTD build now fails to load the extension, so something is broken :(

@juntyr
Copy link
Copy Markdown
Contributor Author

juntyr commented Apr 14, 2026

The RTD build fails to load the extension with

[Error] Plugin '@jupyterlite/litegitpuller:plugin' failed to activate.
	(anonymous function) (jlab_core.cb9a852.js:1:1404219)
[Error] TypeError: undefined is not an object (evaluating 'e.name') — index.es6.js:1933
	(anonymous function) (jlab_core.cb9a852.js:1:1404270)

Do you have any idea about what might be wrong?

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 this pull request may close these issues.

3 participants