Project

General

Profile

Actions

Feature #17415

closed

Create Mountainduck Bookmark files

Added by Daniel Kutyła over 1 year ago. Updated over 1 year ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
Workbench2
Target version:
-
Start date:
04/12/2021
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
-
Release relationship:
Auto

Description

We use a tool called MountainDuck to mount Arvados Collections to the local computers of users. Doing the mounting is for non-technical users a bit tedious and therefore we are looking to optimise this with a convenient functionality inside the workbench.

Workbench2 supports custom file viewers, however these work with urls. What we would need somehow is a custom action for collections (and later also for projects) to generate a XML file with the extension '.duck' and download them. The user would be able to open them which creates the bookmark to the collection (or project.

The basic structure of this file is outlined here:


<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>Protocol</key>
<string>davs</string>
<key>Provider</key>
<string></string>
<key>UUID</key>
<string></string>
<key>Hostname</key>
<string></string>
<key>Port</key>
<string></string>
<key>Username</key>
<string></string>
<key>Path</key>
<string></string>
<key>Access Timestamp</key>
<string></string>
</dict>
</plist>


Subtasks 1 (0 open1 closed)

Task #17524: Review 17415-Mountainduck-Bookmark-filesClosedDaniel Kutyła04/12/2021

Actions
Actions #1

Updated by Daniel Kutyła over 1 year ago

  • Status changed from New to In Progress
Actions #3

Updated by Lucas Di Pentima over 1 year ago

Reviewing arvados-workbench2|37f3a8d

  • As you can see on developer-tests-workbench2: #366 /console -- there're some problems with the cypress test environment.
  • On the Cypress test, could we check that the downloaded .duck file's contents is correct instead of just checking that its size is greater than 50?
  • At file src/store/collections/collection-info-actions.ts
    • Line 38, the collection's name that seems to be retrieved is the one that's being displayed on the collection panel. The problem I'm seeing with this approach is that the "webdav/S3 dialog" can be called from within a context menu inside a project listing, so in that case it will be retrieving an incorrect name if the user selected another collection previously. You can reproduce the issue doing:
      • Create a 'Collection A' collection inside the home project.
      • Create a 'Collection B' collection inside the home project.
      • From within the home project, click on the 'Collection A' collection.
      • Go back to the home project.
      • Right-click on the 'Collection B' collection, ask for the WebDav/S3 dialog and download the mountain duck file.
      • The downloaded file will have the 'Collection A' name.
    • Also line 38: I don't think overriding the typing system by casting to any is convenient.
  • At file src/views-components/webdav-s3-dialog/webdav-s3-dialog.tsx
    • Line 72: Can you explain why you split and re-join the the returned string? If you're providing the template's string on the code, it shouldn't be necessary to do that kind of "cleaning".
    • Line 201: The button's label is not very clear what it downloads. Do you agree that changing it to 'Download Cyber/Mountain Duck bookmark' would be better?
  • Also, have you tried the generated bookmark file? It doesn't work for me on arvbox and ce8i5. I believe it should also work using Cyberduck (free tool from the same team) because I get the same error message when trying both: DNS lookup for davs://user@IP_or_hostname failed.... Some possible problems are:
    • Username, protocol and port number get included on the 'hostname' field.
    • Port number is hardcoded to 443 (it can be different, for example arvbox uses a different one)
    • Password (token) doesn't get included on the file.
Actions #4

Updated by Lucas Di Pentima over 1 year ago

This is the .duck file that I got generated with CyberDuck Mac version 8.7.5:

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
    <key>Protocol</key>
    <string>davs</string>
    <key>Provider</key>
    <string>iterate GmbH</string>
    <key>UUID</key>
    <string>ef96de3e-1bf7-4204-9f60-e2c035a338c1</string>
    <key>Hostname</key>
    <string>ce8i5-4zz18-mx9loldsvg238gk.collections.ce8i5.arvadosapi.com</string>
    <key>Port</key>
    <string>443</string>
    <key>Username</key>
    <string>ldipenti</string>
    <key>Path</key>
    <string>/</string>
    <key>Access Timestamp</key>
    <string>1618242186076</string>
</dict>
</plist>

Note that the Hostname key is populated with just the collection's hostname, and not the entire davs://ldipenti...@ URL.
The password isn't included (although somehow it gets saved and isn't requested), so I'm not sure if we could avoid the user having to enter the password (token) manually. Maybe Cyberduck's docs have some information about that.

Actions #5

Updated by Lucas Di Pentima over 1 year ago

Reviewing changes made on arvados-workbench2|c26242a

Although changes in .duck files made the bookmark work for clusters like ce8i5, it still has the issue of having the 443 port hardcoded and not supporting keep-web collection urls with non-standard ports: for example, if the collection's url is something like https://collection-uuid.hostname:9004, the .duck file's hostname field will be filled with "collection-uuid.hostname:9004" as a hostname, producing a DNS resolution error.

Actions #7

Updated by Lucas Di Pentima over 1 year ago

Tests are failing the same way as described on #note-3: developer-tests-workbench2: #371 /console

Actions #8

Updated by Daniel Kutyła over 1 year ago

New version first commit: https://dev.arvados.org/projects/arvados-workbench-2/repository/revisions/eaf76a93400029d3d788264b829bf878aad1f661
Test run: developer-tests-workbench2: #381

Cypress upgrade to 3.6 in order to fix electron download issue

Actions #9

Updated by Lucas Di Pentima over 1 year ago

Please remove the it.only() call and re-run tests.

Actions #11

Updated by Lucas Di Pentima over 1 year ago

Reviewing arvados-workbench2|0569677b

Lucas Di Pentima wrote:

Reviewing arvados-workbench2|37f3a8d

  • At file src/store/collections/collection-info-actions.ts
    • Line 38, the collection's name that seems to be retrieved is the one that's being displayed on the collection panel. The problem I'm seeing with this approach is that the "webdav/S3 dialog" can be called from within a context menu inside a project listing, so in that case it will be retrieving an incorrect name if the user selected another collection previously. You can reproduce the issue doing:
      • Create a 'Collection A' collection inside the home project.
      • Create a 'Collection B' collection inside the home project.
      • From within the home project, click on the 'Collection A' collection.
      • Go back to the home project.
      • Right-click on the 'Collection B' collection, ask for the WebDav/S3 dialog and download the mountain duck file.
      • The downloaded file will have the 'Collection A' name.

The above issue keeps happening.

  • Also line 38: I don't think overriding the typing system by casting to any is convenient.

This wasn't addressed, is there a reason why?

  • At file src/views-components/webdav-s3-dialog/webdav-s3-dialog.tsx
    • Line 72: Can you explain why you split and re-join the the returned string? If you're providing the template's string on the code, it shouldn't be necessary to do that kind of "cleaning".

This wasn't addressed also, why do you split and then join a string by newline characters if the string is already provided in the code?

  • Line 201: The button's label is not very clear what it downloads. Do you agree that changing it to 'Download Cyber/Mountain Duck bookmark' would be better?

The button still states 'download config' on its label, do you think that's a clear enough name?

  • The cypress test looks good. The XML file decoding code seems a little convoluted to me, it would be nice to have at least a comment explaining what is doing, for future readers.
  • I think you should commit the updated yarn.lock file.
  • The non-default port fix seems to be correct, but there's also a related issue with hostnames: When wildcard hostnames aren't set and only IPs are used (for example, the arvbox use case), the bookmark file should include a Path key with the value /c=<collection uuid or pdh> as described on https://doc.arvados.org/v2.1/api/keep-webdav.html

Here's an example that works on my arvbox instance:

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
    <key>Protocol</key>
    <string>davs</string>
    <key>Provider</key>
    <string>iterate GmbH</string>
    <key>UUID</key>
    <string>x240b-4zz18-cx9p60347rmsyoi</string>
    <key>Hostname</key>
    <string>10.1.1.32</string>
    <key>Port</key>
    <string>9004</string>
    <key>Username</key>
    <string>user</string>
    <key>Path</key>
    <string>/c=x240b-4zz18-cx9p60347rmsyoi</string>
    <key>Access Timestamp</key>
    <string>1618953853339</string>
    <key>Labels</key>
    <array>
    </array>
</dict>
</plist>
Actions #13

Updated by Daniel Kutyła over 1 year ago

This wasn't addressed, is there a reason why?

At file src/views-components/webdav-s3-dialog/webdav-s3-dialog.tsx
Line 72: Can you explain why you split and re-join the the returned string? If you're providing the template's string on the code, it shouldn't be necessary to do that kind of "cleaning".

It is used to remove windows endlines which cause file not being parsed properly

Actions #15

Updated by Lucas Di Pentima over 1 year ago

Daniel Kutyła wrote:

This wasn't addressed, is there a reason why?

At file src/views-components/webdav-s3-dialog/webdav-s3-dialog.tsx
Line 72: Can you explain why you split and re-join the the returned string? If you're providing the template's string on the code, it shouldn't be necessary to do that kind of "cleaning".

It is used to remove windows endlines which cause file not being parsed properly

I'm not sure what you mean by this. I've tried removing the splitting+re-joining calls from the code and the generated file worked just fine.

Just a couple more comments:
  • At file src/views-components/webdav-s3-dialog/webdav-s3-dialog.tsx Line 48 there's some console logging that I think you didn't meant to leave.
  • At file cypress/integration/collection.spec.js Line 43 there's a cy.doSearch(collectionUUID) call that can be replaced with the recently merged cy.goToPath('/collections/collectionUUID'), it will be faster because it uses the app's router instead of doing a wb2 search.

The rest LGTM.

Actions #16

Updated by Daniel Kutyła over 1 year ago

  • % Done changed from 0 to 100
  • Status changed from In Progress to Resolved
Actions #17

Updated by Peter Amstutz over 1 year ago

  • Release set to 38
Actions

Also available in: Atom PDF