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 file openers by to accept URLs #5885

Closed
saulshanabrook opened this issue Jan 19, 2019 · 12 comments · Fixed by #7596
Closed

Allow file openers by to accept URLs #5885

saulshanabrook opened this issue Jan 19, 2019 · 12 comments · Fixed by #7596
Labels
enhancement pkg:docregistry status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion.
Milestone

Comments

@saulshanabrook
Copy link
Member

I would like to register an opener for files that gets called with the URL of them instead of their contents.

The goal is for the user to click "Open With > My Custom Opener" and for me to get the URL of the file without loading the contents.

I would like to care of loading the contents myself, if/when I need them. Many applications do the loading themselves, so it is wasteful to load the twice.

I need this for my work on the databus, to allow users to open files with the databus. #5857

@saulshanabrook
Copy link
Member Author

cc @ian-r-rose @ellisonbg @Carreau

@jasongrout
Copy link
Contributor

This sort of thing is explored a lot in #4902

@ian-r-rose
Copy link
Member

As well as in #2941. Incidentally, I recently moved both of those off the 1.0 milestone

@saulshanabrook
Copy link
Member Author

Thanks @jasongrout and @ian-r-rose for those links.

GET ./api/contents/<file> code:

https://github.com/jupyter/jupyter_server/blob/ad57725ca84701228dc90825f09a20ce14b68e5b/jupyter_server/services/contents/handlers.py#L90-L115

GET ./files/<file> code:

https://github.com/jupyter/jupyter_server/blob/ad57725ca84701228dc90825f09a20ce14b68e5b/jupyter_server/files/handlers.py#L39-L81

So the contents API doesn't have a way to get back metadata about a file without getting the contents. This could be added, by adding another format called empty. However, not sure we need to do this, at least at first.

Seems like MVP would just be for a file handler to get the ./files/<...> URL passed in for the relevant file, and it could fetch it. I will try that!

@ian-r-rose
Copy link
Member

ian-r-rose commented Jan 19, 2019

The contents API can get back the metadata via the contents API by passing the contents=false query parameter (which is shown in your first link). We do that in a few places to check for file existence.

@ian-r-rose
Copy link
Member

Here are a couple of examples to get us started:

app.commands.addCommand(CommandIDs.handleLink, {
label: 'Handle Local Link',
execute: args => {
const path = args['path'] as string | undefined | null;
const id = args['id'] as string | undefined | null;
if (!path) {
return;
}
// First check if the path exists on the server.
return docManager.services.contents
.get(path, { content: false })
.then(() => {
// Open the link with the default rendered widget factory,
// if applicable.
const factory = docManager.registry.defaultRenderedWidgetFactory(
path
);
const widget = docManager.openOrReveal(path, factory.name);
// Handle the hash if one has been provided.
if (widget && id) {
widget.setFragment(id);
}
});
}
});

commands.addCommand(CommandIDs.open, {
execute: args => {
const path =
typeof args['path'] === 'undefined' ? '' : (args['path'] as string);
const factory = (args['factory'] as string) || void 0;
const kernel = (args['kernel'] as Kernel.IModel) || void 0;
const options =
(args['options'] as DocumentRegistry.IOpenOptions) || void 0;
return docManager.services.contents
.get(path, { content: false })
.then(() => docManager.openOrReveal(path, factory, kernel, options));
},
icon: args => (args['icon'] as string) || '',
label: args => (args['label'] || args['factory']) as string,
mnemonic: args => (args['mnemonic'] as number) || -1
});

@saulshanabrook
Copy link
Member Author

saulshanabrook commented Jan 19, 2019

It seems like this is possible now. I was able to add an item to the open with menu that allows me to get the URL of the file I opened without actually loading it:

const url = context.urlResolver.getDownloadUrl(context.path);

EDIT: I was wrong, this gets the contents currently

@jasongrout jasongrout added this to the Future milestone Jan 25, 2019
@robyww
Copy link

robyww commented May 8, 2019

Is there any progress on telling lab to not send the contents in an opener? Getting a url is nice but if a large file is still being transferred it does not help that much.

We get the model in the browser and then turn around and send it to another server. I would prefer to just get the just get the file name and the tell a server extension to post it. That way the data does not have to be sent twice. For large files this is a really big deal.

@jasongrout
Copy link
Contributor

As far as I know, there hasn't been any progress on this beyond the above comments (@saulshanabrook is probably the most likely one to have made progress on it).

@robyww
Copy link

robyww commented May 8, 2019

Today I am trying an approach of just getting the filename and telling a server extension that I wrote to upload the file. I am ignoring the model altogether. It is radically faster. I am sure the data is still being sent to the browser but since it is not accessed in the javascript thread it does not appear the affect performance.

this.renderModel(context._path)   // <-- before this would happen in context.ready.then() promise
context.model.contentChanged.connect( this.renderModel, this );
context.fileChanged.connect( this.renderModel, this );

The upload js code, uses a sendToFirefly endpoint the :

function tellLabToloadFileToFireflyServer( filename, firefly) {
    const UL_URL= window.document.location.href+ `/sendToFirefly?path=${filename}`;
    return firefly.util.fetchUrl(UL_URL, { method: 'GET' },false, false);
}

@telamonian
Copy link
Member

I think I was treading some similar ground in a recent discussion on how to fix some file path handling stuff:

#6905 (comment)

My vote would be to revert and then add an optional url field to Contents.IModel that gets consumed by cd (and elsewhere) as is. That would (optionally) decouple filebrowser's concept of directories from any underlying representation, and make it much easier to do creative things with filebrowser and the related machinery.

@jasongrout
Copy link
Contributor

It seems like this is possible now. I was able to add an item to the open with menu that allows me to get the URL of the file I opened without actually loading it:

This is coming up in conversation right now. It appears that (at least now) the context object asynchronously fetches the content, so it will appear at first that the content isn't there, but when the context ready promise is resolved, the content should be there.

@github-actions github-actions bot added the status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label Sep 21, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement pkg:docregistry 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.

6 participants