Skip to content
This repository has been archived by the owner on Jun 4, 2024. It is now read-only.

CSV: let users upload a CSV file #383

Closed
n-riesco opened this issue Feb 21, 2018 · 4 comments · Fixed by #385
Closed

CSV: let users upload a CSV file #383

n-riesco opened this issue Feb 21, 2018 · 4 comments · Fixed by #385

Comments

@n-riesco
Copy link
Contributor

n-riesco commented Feb 21, 2018

A continuation of work in PR #349.

@jackparmer @jody @tarzzz Below is a discussion of two issues with the CSV connector that will affect the onprem setup.


Up to now, connections in Falcon are permanent. That is, when a user connects to a database, the connection is stored in ~/.plotly/connector/connections.yaml. If we close Falcon and we open it again, the connections in ~/.plotly/connector/connections.yaml are restored.

This behaviour wouldn't be possible, unless a copy of the uploaded CSV file is stored in the Falcon server. I'm sure we don't want the Falcon server to become an online file storage.

For this reason, I'm planning to work around it, at the start up of Falcon, by removing from ~/.plotly/connector/connections.yaml all the connections corresponding to uploaded CSV files.

Does that sound OK?


An alternative approach would be for the CSV connector to accept file:// URLs. But this would only make sense for the desktop app.


Another issue that I haven't addressed in #349 is that, users can try to upload a CSV file so large that the Falcon server runs out of memory.

I guess we could have a setting that limits the total amount of memory taken by CSV files.

What do you think about having this setting?

@jackparmer
Copy link
Contributor

I'm planning to work around it, at the start up of Falcon, by removing from ~/.plotly/connector/connections.yaml all the connections corresponding to uploaded CSV files.

👍 Sounds right

An alternative approach would be for the CSV connector to accept file:// URLs.

Too complicated. This should be a normal file upload that any Excel user could figure out.

I guess we could have a setting that limits the total amount of memory taken by CSV files.

We do this Chart Studio too.

@n-riesco
Copy link
Contributor Author

n-riesco commented Feb 27, 2018

@nicolaskruchten @jackparmer I've just merged #385 and now users can drag'n'drop a file onto the CSV connector.

I've tested the connector on 4GB VM and it can cope with a 40MB CSV file. But a 120MB CSV file crashes Falcon's server.

@nicolaskruchten
Copy link
Contributor

Super cool! I played with it a bit and just have one bit of feedback... When I drop my CSV file, I have to hit "connect" and then it switches to a view where it says "connected" greyed out and "edit credentials" active. I then have to hit "Query" to actually see/chart the data. This makes a lot of sense in a database context but less in a file context. Maybe a bit of text saying "Connection succeeded! Click here to go to query your data, or click here to edit your credentials/change file" ?

@n-riesco
Copy link
Contributor Author

@nicolaskruchten We're in the process of refactoring the UI (see #362). One of the changes I've proposed is actually meant to help us simplify UI and have one button instead of two. We'll address your suggestion then.

Another thing we could do to make the UX more fluent is to open the query tab automatically when the connection succeeds.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants