Skip to content
This repository was archived by the owner on Feb 8, 2024. It is now read-only.

SharedDirectoryListResponse#1000

Merged
ibeckermayer merged 50 commits intomasterfrom
isaiah/sd-list-response
Jul 27, 2022
Merged

SharedDirectoryListResponse#1000
ibeckermayer merged 50 commits intomasterfrom
isaiah/sd-list-response

Conversation

@ibeckermayer
Copy link
Copy Markdown

Adds listContents to get the information about each item in a directory and send it back in the fsoList of the SharedDirectoryListResponse.

I initially tried to create a version of listContents which gathered all of the promises in a list and then returned them wrapped in a Promise.all. However with this structure, sharing a directory with a large number of items (like the node_modules directory of this repo after yarn install) would cause the system to freeze up for a while.

I think its a similar problem to the one I encountered way back when working on the display pipeline, namely that piling up a lot of asynchronous calls clogs up the microtask queue and can make performance appear worse, because new regular tasks can't be executed while all the microtasks are processed.

Performance still isn't fantastic when sharing a large directory, but its good enough to be useable.

Requires backport to v9/v10

How to test manually

webapps

  1. Change the value below to true
    enableDirectorySharing: false, // note to reviewers: should be false in any PRs.
  2. run the development server like yarn start-teleport --target=https://ec2-35-171-27-185.compute-1.amazonaws.com/
  3. See https://gravitational.slack.com/archives/C02DQ1C2BMW/p1657807702085829 for a username/password combo

What to expect

Go to the top right ... menu and click Share Directory

image

(X-out of the "uncaught exception" screen if you get it). The directory you select should show up in the File Explorer as a shared drive

Screen Shot 2022-07-19 at 14 13 45

The drive should be navigable as well should any folders within it

image

Trying to open a file should result in a

image

Isaiah Becker-Mayer added 30 commits July 6, 2022 15:53
@JanKaczmarkiewicz
Copy link
Copy Markdown
Contributor

JanKaczmarkiewicz commented Jul 20, 2022

I initially tried to create a version of listContents which gathered all of the promises in a list and then returned them wrapped in a Promise.all. However with this structure, sharing a directory with a large number of items (like the node_modules directory of this repo after yarn install) would cause the system to freeze up for a while.

I think its a similar problem to the one I encountered way back when working on the display pipeline, namely that piling up a lot of asynchronous calls clogs up the microtask queue and can make performance appear worse, because new regular tasks can't be executed while all the microtasks are processed.

We could use a promise pool eg: https://stackoverflow.com/a/63620273/12860195

@ibeckermayer
Copy link
Copy Markdown
Author

ibeckermayer commented Jul 20, 2022

We could use a promise pool eg: https://stackoverflow.com/a/63620273/12860195

@JanKaczmarkiewicz very clever idea and implementation. I tried to copy it in another branch isaiah/sd-list-response-poolPromises. Our system is complicated by the fact that dir.values() gives us an AsyncIterableIterator, so we are most naturally inclined to create an AsyncGenerator. When I attempted to run the code as written, enumerating the shared directory broke the tab.

There may yet be a way to use this, but I'm going to kick the can down the road. Added this discussion thread to the relevant issue: gravitational/teleport#21073

@ibeckermayer ibeckermayer changed the base branch from isaiah/sd-list-request to master July 27, 2022 22:39
@ibeckermayer ibeckermayer enabled auto-merge (squash) July 27, 2022 22:48
@ibeckermayer ibeckermayer merged commit 3687cff into master Jul 27, 2022
ibeckermayer pushed a commit that referenced this pull request Aug 23, 2022
ibeckermayer pushed a commit that referenced this pull request Aug 24, 2022
* `SharedDirectoryInfoResponse` (#996)

* `SharedDirectoryListRequest` (#999)

* `SharedDirectoryListResponse` (#1000)

* `SharedDirectoryReadRequest` (#1003)

* `SharedDirectoryReadResponse` (#1005)

* `SharedDirectoryWriteRequest` (#1007)

* `SharedDirectoryWriteResponse` (#1008)

* Tidy up `sharedDirectoryManager` (#1010)

* `SharedDirectoryMoveRequest` (#1045)

* `SharedDirectoryMoveResponse` (#1074)

* `SharedDirectoryCreateRequest` and `SharedDirectoryCreateResponse` (#1090)

* SharedDirectoryDeleteRequest and SharedDirectoryDeleteResponse (#1096)

* Add warning dialog for unsupported browsers for directory sharing (#1110)

* updates yarn.lock
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants