CSV connector by n-riesco · Pull Request #375 · plotly/falcon

n-riesco · 2018-02-15T18:30:32Z

Do not merge before test: replace chai-spies with sinon #374

shannonlal · 2018-02-16T11:04:31Z

+const connectionData = {};
+
+// define type error thrown by this connector
+export function CSVError(url, errors) {


You created your own CSV Error with specific information. I am just trying to clarify why you needed your own error type. How will this be handled on the client?

shannonlal · 2018-02-16T11:06:22Z

@n-riesco I did a review this and everything looks good with the exception of creating your own error. I just wanted to better understand this and how these errors will be displayed to the client.

n-riesco · 2018-02-16T13:12:09Z

@shannonlal At the moment, the frontend only shows error.message.

The reason I defined a new error type was because papaparse returns a list of errors and line numbers (and I wanted to keep track of them). At the moment, I'm setting error.message to errors[0].message, but this may change in the future.

I don't know yet if I'm going to stick to papaparse, because it fails to parse many CSV files I've tested (I'm still fiddling with the settings).

The PR is still work in progress. It works, but it still has a few rough corners.

* This connector takes a URL as input, downloads the URL and parses it as a CSV file into a Javascript object that can be queried using alaSQL. * Added test specs and the script `test-unit-csv` to run them. * Updated `Setting.react.js` so that future connectors don't need to update it (updated `NEW_CONNECTION.md` accordingly). * TODO (in future PRs): - `papaparse` parses CSV data only into numbers and strings. Add option to let the user set a date format for parsing. - let the user assign a table name - let the user upload multiple CSV files Closesplotly#349

n-riesco · 2018-02-16T19:09:05Z

@shannonlal I've just fixed the last few rough corners with this PR. I think the PR is ready for review. Would you like to review it?

shannonlal · 2018-02-16T19:59:40Z

@n-riesco Yes I will have time to look this over on the weekend

shannonlal

All the comments I added are small and minor. Everything looks good it was just some minor suggestions. I think it is ready for a dancer. Just let me know if you think my changes make sense or if you want to merge the changes as is.

shannonlal · 2018-02-17T19:02:11Z

+ if (cell) return type(cell);
+ }
+
+ return 'String';


Maybe just add a quick comment to explain that default is string. Or add to JSDocs to function

shannonlal · 2018-02-17T19:03:14Z

+CSVError.prototype.constructor = CSVError;
+
+// the data parsed from CSV files is stored here
+const connectionData = {};


In #362 One of the comments was to start using JSDocs style for our code. Should we be doing this on the backend as well?

shannonlal · 2018-02-17T19:04:16Z

+ connectionData[connection.database] = data;
+}
+
+export function connect(connection) {


If the url is bad, how will this be presented to the user on the Settings UI? will fetch throw an error and this will be gracefully passed to the UI on its own?

Same behaviour for all the connectors, i.e.: fetch throws, routes.js catches the exception and replies to the connection request with a 500 status and the body {error: message: error.message}}.

shannonlal · 2018-02-17T19:05:24Z

+
+export function tables() {
+ // To take advantage of alaSQL's parser, the table is named '?'
+ return Promise.resolve(['?']);


Should we be using '?' or a const CSV_TABLE_NAME = '?'. This way if it changes in the future you don't have to string replace to make sure we find all changes

n-riesco · 2018-02-15T18:30:32Z

Do not merge before test: replace chai-spies with sinon #374

shannonlal · 2018-02-16T11:04:31Z

+const connectionData = {};
+
+// define type error thrown by this connector
+export function CSVError(url, errors) {


You created your own CSV Error with specific information. I am just trying to clarify why you needed your own error type. How will this be handled on the client?

shannonlal · 2018-02-16T11:06:22Z

@n-riesco I did a review this and everything looks good with the exception of creating your own error. I just wanted to better understand this and how these errors will be displayed to the client.

n-riesco · 2018-02-16T13:12:09Z

@shannonlal At the moment, the frontend only shows error.message.

The reason I defined a new error type was because papaparse returns a list of errors and line numbers (and I wanted to keep track of them). At the moment, I'm setting error.message to errors[0].message, but this may change in the future.

I don't know yet if I'm going to stick to papaparse, because it fails to parse many CSV files I've tested (I'm still fiddling with the settings).

The PR is still work in progress. It works, but it still has a few rough corners.

* This connector takes a URL as input, downloads the URL and parses it as a CSV file into a Javascript object that can be queried using alaSQL. * Added test specs and the script `test-unit-csv` to run them. * Updated `Setting.react.js` so that future connectors don't need to update it (updated `NEW_CONNECTION.md` accordingly). * TODO (in future PRs): - `papaparse` parses CSV data only into numbers and strings. Add option to let the user set a date format for parsing. - let the user assign a table name - let the user upload multiple CSV files Closesplotly#349

n-riesco · 2018-02-16T19:09:05Z

@shannonlal I've just fixed the last few rough corners with this PR. I think the PR is ready for review. Would you like to review it?

shannonlal · 2018-02-16T19:59:40Z

@n-riesco Yes I will have time to look this over on the weekend

shannonlal

All the comments I added are small and minor. Everything looks good it was just some minor suggestions. I think it is ready for a dancer. Just let me know if you think my changes make sense or if you want to merge the changes as is.

shannonlal · 2018-02-17T19:02:11Z

+ if (cell) return type(cell);
+ }
+
+ return 'String';


Maybe just add a quick comment to explain that default is string. Or add to JSDocs to function

shannonlal · 2018-02-17T19:03:14Z

+CSVError.prototype.constructor = CSVError;
+
+// the data parsed from CSV files is stored here
+const connectionData = {};


In #362 One of the comments was to start using JSDocs style for our code. Should we be doing this on the backend as well?

shannonlal · 2018-02-17T19:04:16Z

+ connectionData[connection.database] = data;
+}
+
+export function connect(connection) {


If the url is bad, how will this be presented to the user on the Settings UI? will fetch throw an error and this will be gracefully passed to the UI on its own?

Same behaviour for all the connectors, i.e.: fetch throws, routes.js catches the exception and replies to the connection request with a 500 status and the body {error: message: error.message}}.

shannonlal · 2018-02-17T19:05:24Z

+
+export function tables() {
+ // To take advantage of alaSQL's parser, the table is named '?'
+ return Promise.resolve(['?']);


Should we be using '?' or a const CSV_TABLE_NAME = '?'. This way if it changes in the future you don't have to string replace to make sure we find all changes

n-riesco · 2018-02-15T18:30:32Z

Do not merge before test: replace chai-spies with sinon #374

shannonlal · 2018-02-16T11:04:31Z

+const connectionData = {};
+
+// define type error thrown by this connector
+export function CSVError(url, errors) {


You created your own CSV Error with specific information. I am just trying to clarify why you needed your own error type. How will this be handled on the client?

shannonlal · 2018-02-16T11:06:22Z

@n-riesco I did a review this and everything looks good with the exception of creating your own error. I just wanted to better understand this and how these errors will be displayed to the client.

n-riesco · 2018-02-16T13:12:09Z

@shannonlal At the moment, the frontend only shows error.message.

The reason I defined a new error type was because papaparse returns a list of errors and line numbers (and I wanted to keep track of them). At the moment, I'm setting error.message to errors[0].message, but this may change in the future.

I don't know yet if I'm going to stick to papaparse, because it fails to parse many CSV files I've tested (I'm still fiddling with the settings).

The PR is still work in progress. It works, but it still has a few rough corners.

* This connector takes a URL as input, downloads the URL and parses it as a CSV file into a Javascript object that can be queried using alaSQL. * Added test specs and the script `test-unit-csv` to run them. * Updated `Setting.react.js` so that future connectors don't need to update it (updated `NEW_CONNECTION.md` accordingly). * TODO (in future PRs): - `papaparse` parses CSV data only into numbers and strings. Add option to let the user set a date format for parsing. - let the user assign a table name - let the user upload multiple CSV files Closesplotly#349

n-riesco · 2018-02-16T19:09:05Z

@shannonlal I've just fixed the last few rough corners with this PR. I think the PR is ready for review. Would you like to review it?

shannonlal · 2018-02-16T19:59:40Z

@n-riesco Yes I will have time to look this over on the weekend

shannonlal

All the comments I added are small and minor. Everything looks good it was just some minor suggestions. I think it is ready for a dancer. Just let me know if you think my changes make sense or if you want to merge the changes as is.

shannonlal · 2018-02-17T19:02:11Z

+ if (cell) return type(cell);
+ }
+
+ return 'String';


Maybe just add a quick comment to explain that default is string. Or add to JSDocs to function

shannonlal · 2018-02-17T19:03:14Z

+CSVError.prototype.constructor = CSVError;
+
+// the data parsed from CSV files is stored here
+const connectionData = {};


In #362 One of the comments was to start using JSDocs style for our code. Should we be doing this on the backend as well?

shannonlal · 2018-02-17T19:04:16Z

+ connectionData[connection.database] = data;
+}
+
+export function connect(connection) {


If the url is bad, how will this be presented to the user on the Settings UI? will fetch throw an error and this will be gracefully passed to the UI on its own?

Same behaviour for all the connectors, i.e.: fetch throws, routes.js catches the exception and replies to the connection request with a 500 status and the body {error: message: error.message}}.

shannonlal · 2018-02-17T19:05:24Z

+
+export function tables() {
+ // To take advantage of alaSQL's parser, the table is named '?'
+ return Promise.resolve(['?']);


Should we be using '?' or a const CSV_TABLE_NAME = '?'. This way if it changes in the future you don't have to string replace to make sure we find all changes

n-riesco · 2018-02-15T18:30:32Z

Do not merge before test: replace chai-spies with sinon #374

shannonlal · 2018-02-16T11:04:31Z

+const connectionData = {};
+
+// define type error thrown by this connector
+export function CSVError(url, errors) {


You created your own CSV Error with specific information. I am just trying to clarify why you needed your own error type. How will this be handled on the client?

shannonlal · 2018-02-16T11:06:22Z

@n-riesco I did a review this and everything looks good with the exception of creating your own error. I just wanted to better understand this and how these errors will be displayed to the client.

n-riesco · 2018-02-16T13:12:09Z

@shannonlal At the moment, the frontend only shows error.message.

The reason I defined a new error type was because papaparse returns a list of errors and line numbers (and I wanted to keep track of them). At the moment, I'm setting error.message to errors[0].message, but this may change in the future.

I don't know yet if I'm going to stick to papaparse, because it fails to parse many CSV files I've tested (I'm still fiddling with the settings).

The PR is still work in progress. It works, but it still has a few rough corners.

* This connector takes a URL as input, downloads the URL and parses it as a CSV file into a Javascript object that can be queried using alaSQL. * Added test specs and the script `test-unit-csv` to run them. * Updated `Setting.react.js` so that future connectors don't need to update it (updated `NEW_CONNECTION.md` accordingly). * TODO (in future PRs): - `papaparse` parses CSV data only into numbers and strings. Add option to let the user set a date format for parsing. - let the user assign a table name - let the user upload multiple CSV files Closesplotly#349

n-riesco · 2018-02-16T19:09:05Z

@shannonlal I've just fixed the last few rough corners with this PR. I think the PR is ready for review. Would you like to review it?

shannonlal · 2018-02-16T19:59:40Z

@n-riesco Yes I will have time to look this over on the weekend

shannonlal

All the comments I added are small and minor. Everything looks good it was just some minor suggestions. I think it is ready for a dancer. Just let me know if you think my changes make sense or if you want to merge the changes as is.

shannonlal · 2018-02-17T19:02:11Z

+ if (cell) return type(cell);
+ }
+
+ return 'String';


Maybe just add a quick comment to explain that default is string. Or add to JSDocs to function

shannonlal · 2018-02-17T19:03:14Z

+CSVError.prototype.constructor = CSVError;
+
+// the data parsed from CSV files is stored here
+const connectionData = {};


In #362 One of the comments was to start using JSDocs style for our code. Should we be doing this on the backend as well?

shannonlal · 2018-02-17T19:04:16Z

+ connectionData[connection.database] = data;
+}
+
+export function connect(connection) {


If the url is bad, how will this be presented to the user on the Settings UI? will fetch throw an error and this will be gracefully passed to the UI on its own?

Same behaviour for all the connectors, i.e.: fetch throws, routes.js catches the exception and replies to the connection request with a 500 status and the body {error: message: error.message}}.

shannonlal · 2018-02-17T19:05:24Z

+
+export function tables() {
+ // To take advantage of alaSQL's parser, the table is named '?'
+ return Promise.resolve(['?']);


Should we be using '?' or a const CSV_TABLE_NAME = '?'. This way if it changes in the future you don't have to string replace to make sure we find all changes

n-riesco · 2018-02-15T18:30:32Z

Do not merge before test: replace chai-spies with sinon #374

shannonlal · 2018-02-16T11:04:31Z

+const connectionData = {};
+
+// define type error thrown by this connector
+export function CSVError(url, errors) {


You created your own CSV Error with specific information. I am just trying to clarify why you needed your own error type. How will this be handled on the client?

shannonlal · 2018-02-16T11:06:22Z

@n-riesco I did a review this and everything looks good with the exception of creating your own error. I just wanted to better understand this and how these errors will be displayed to the client.

n-riesco · 2018-02-16T13:12:09Z

@shannonlal At the moment, the frontend only shows error.message.

The reason I defined a new error type was because papaparse returns a list of errors and line numbers (and I wanted to keep track of them). At the moment, I'm setting error.message to errors[0].message, but this may change in the future.

I don't know yet if I'm going to stick to papaparse, because it fails to parse many CSV files I've tested (I'm still fiddling with the settings).

The PR is still work in progress. It works, but it still has a few rough corners.

* This connector takes a URL as input, downloads the URL and parses it as a CSV file into a Javascript object that can be queried using alaSQL. * Added test specs and the script `test-unit-csv` to run them. * Updated `Setting.react.js` so that future connectors don't need to update it (updated `NEW_CONNECTION.md` accordingly). * TODO (in future PRs): - `papaparse` parses CSV data only into numbers and strings. Add option to let the user set a date format for parsing. - let the user assign a table name - let the user upload multiple CSV files Closesplotly#349

n-riesco · 2018-02-16T19:09:05Z

@shannonlal I've just fixed the last few rough corners with this PR. I think the PR is ready for review. Would you like to review it?

shannonlal · 2018-02-16T19:59:40Z

@n-riesco Yes I will have time to look this over on the weekend

shannonlal

All the comments I added are small and minor. Everything looks good it was just some minor suggestions. I think it is ready for a dancer. Just let me know if you think my changes make sense or if you want to merge the changes as is.

shannonlal · 2018-02-17T19:02:11Z

+ if (cell) return type(cell);
+ }
+
+ return 'String';


Maybe just add a quick comment to explain that default is string. Or add to JSDocs to function

shannonlal · 2018-02-17T19:03:14Z

+CSVError.prototype.constructor = CSVError;
+
+// the data parsed from CSV files is stored here
+const connectionData = {};


In #362 One of the comments was to start using JSDocs style for our code. Should we be doing this on the backend as well?

shannonlal · 2018-02-17T19:04:16Z

+ connectionData[connection.database] = data;
+}
+
+export function connect(connection) {


If the url is bad, how will this be presented to the user on the Settings UI? will fetch throw an error and this will be gracefully passed to the UI on its own?

Same behaviour for all the connectors, i.e.: fetch throws, routes.js catches the exception and replies to the connection request with a 500 status and the body {error: message: error.message}}.

shannonlal · 2018-02-17T19:05:24Z

+
+export function tables() {
+ // To take advantage of alaSQL's parser, the table is named '?'
+ return Promise.resolve(['?']);


Should we be using '?' or a const CSV_TABLE_NAME = '?'. This way if it changes in the future you don't have to string replace to make sure we find all changes

n-riesco · 2018-02-15T18:30:32Z

Do not merge before test: replace chai-spies with sinon #374

shannonlal · 2018-02-16T11:04:31Z

+const connectionData = {};
+
+// define type error thrown by this connector
+export function CSVError(url, errors) {


You created your own CSV Error with specific information. I am just trying to clarify why you needed your own error type. How will this be handled on the client?

shannonlal · 2018-02-16T11:06:22Z

@n-riesco I did a review this and everything looks good with the exception of creating your own error. I just wanted to better understand this and how these errors will be displayed to the client.

n-riesco · 2018-02-16T13:12:09Z

@shannonlal At the moment, the frontend only shows error.message.

The reason I defined a new error type was because papaparse returns a list of errors and line numbers (and I wanted to keep track of them). At the moment, I'm setting error.message to errors[0].message, but this may change in the future.

I don't know yet if I'm going to stick to papaparse, because it fails to parse many CSV files I've tested (I'm still fiddling with the settings).

The PR is still work in progress. It works, but it still has a few rough corners.

* This connector takes a URL as input, downloads the URL and parses it as a CSV file into a Javascript object that can be queried using alaSQL. * Added test specs and the script `test-unit-csv` to run them. * Updated `Setting.react.js` so that future connectors don't need to update it (updated `NEW_CONNECTION.md` accordingly). * TODO (in future PRs): - `papaparse` parses CSV data only into numbers and strings. Add option to let the user set a date format for parsing. - let the user assign a table name - let the user upload multiple CSV files Closesplotly#349

n-riesco · 2018-02-16T19:09:05Z

@shannonlal I've just fixed the last few rough corners with this PR. I think the PR is ready for review. Would you like to review it?

shannonlal · 2018-02-16T19:59:40Z

@n-riesco Yes I will have time to look this over on the weekend

shannonlal

All the comments I added are small and minor. Everything looks good it was just some minor suggestions. I think it is ready for a dancer. Just let me know if you think my changes make sense or if you want to merge the changes as is.

shannonlal · 2018-02-17T19:02:11Z

+ if (cell) return type(cell);
+ }
+
+ return 'String';


Maybe just add a quick comment to explain that default is string. Or add to JSDocs to function

shannonlal · 2018-02-17T19:03:14Z

+CSVError.prototype.constructor = CSVError;
+
+// the data parsed from CSV files is stored here
+const connectionData = {};


In #362 One of the comments was to start using JSDocs style for our code. Should we be doing this on the backend as well?

shannonlal · 2018-02-17T19:04:16Z

+ connectionData[connection.database] = data;
+}
+
+export function connect(connection) {


If the url is bad, how will this be presented to the user on the Settings UI? will fetch throw an error and this will be gracefully passed to the UI on its own?

Same behaviour for all the connectors, i.e.: fetch throws, routes.js catches the exception and replies to the connection request with a 500 status and the body {error: message: error.message}}.

shannonlal · 2018-02-17T19:05:24Z

+
+export function tables() {
+ // To take advantage of alaSQL's parser, the table is named '?'
+ return Promise.resolve(['?']);


Should we be using '?' or a const CSV_TABLE_NAME = '?'. This way if it changes in the future you don't have to string replace to make sure we find all changes

n-riesco · 2018-02-15T18:30:32Z

Do not merge before test: replace chai-spies with sinon #374

shannonlal · 2018-02-16T11:04:31Z

+const connectionData = {};
+
+// define type error thrown by this connector
+export function CSVError(url, errors) {


You created your own CSV Error with specific information. I am just trying to clarify why you needed your own error type. How will this be handled on the client?

shannonlal · 2018-02-16T11:06:22Z

@n-riesco I did a review this and everything looks good with the exception of creating your own error. I just wanted to better understand this and how these errors will be displayed to the client.

n-riesco · 2018-02-16T13:12:09Z

@shannonlal At the moment, the frontend only shows error.message.

The reason I defined a new error type was because papaparse returns a list of errors and line numbers (and I wanted to keep track of them). At the moment, I'm setting error.message to errors[0].message, but this may change in the future.

I don't know yet if I'm going to stick to papaparse, because it fails to parse many CSV files I've tested (I'm still fiddling with the settings).

The PR is still work in progress. It works, but it still has a few rough corners.

* This connector takes a URL as input, downloads the URL and parses it as a CSV file into a Javascript object that can be queried using alaSQL. * Added test specs and the script `test-unit-csv` to run them. * Updated `Setting.react.js` so that future connectors don't need to update it (updated `NEW_CONNECTION.md` accordingly). * TODO (in future PRs): - `papaparse` parses CSV data only into numbers and strings. Add option to let the user set a date format for parsing. - let the user assign a table name - let the user upload multiple CSV files Closesplotly#349

n-riesco · 2018-02-16T19:09:05Z

@shannonlal I've just fixed the last few rough corners with this PR. I think the PR is ready for review. Would you like to review it?

shannonlal · 2018-02-16T19:59:40Z

@n-riesco Yes I will have time to look this over on the weekend

shannonlal

All the comments I added are small and minor. Everything looks good it was just some minor suggestions. I think it is ready for a dancer. Just let me know if you think my changes make sense or if you want to merge the changes as is.

shannonlal · 2018-02-17T19:02:11Z

+ if (cell) return type(cell);
+ }
+
+ return 'String';


Maybe just add a quick comment to explain that default is string. Or add to JSDocs to function

shannonlal · 2018-02-17T19:03:14Z

+CSVError.prototype.constructor = CSVError;
+
+// the data parsed from CSV files is stored here
+const connectionData = {};


In #362 One of the comments was to start using JSDocs style for our code. Should we be doing this on the backend as well?

shannonlal · 2018-02-17T19:04:16Z

+ connectionData[connection.database] = data;
+}
+
+export function connect(connection) {


If the url is bad, how will this be presented to the user on the Settings UI? will fetch throw an error and this will be gracefully passed to the UI on its own?

Same behaviour for all the connectors, i.e.: fetch throws, routes.js catches the exception and replies to the connection request with a 500 status and the body {error: message: error.message}}.

shannonlal · 2018-02-17T19:05:24Z

+
+export function tables() {
+ // To take advantage of alaSQL's parser, the table is named '?'
+ return Promise.resolve(['?']);


Should we be using '?' or a const CSV_TABLE_NAME = '?'. This way if it changes in the future you don't have to string replace to make sure we find all changes

n-riesco · 2018-02-15T18:30:32Z

Do not merge before test: replace chai-spies with sinon #374

shannonlal · 2018-02-16T11:04:31Z

+const connectionData = {};
+
+// define type error thrown by this connector
+export function CSVError(url, errors) {


You created your own CSV Error with specific information. I am just trying to clarify why you needed your own error type. How will this be handled on the client?

shannonlal · 2018-02-16T11:06:22Z

@n-riesco I did a review this and everything looks good with the exception of creating your own error. I just wanted to better understand this and how these errors will be displayed to the client.

n-riesco · 2018-02-16T13:12:09Z

@shannonlal At the moment, the frontend only shows error.message.

The reason I defined a new error type was because papaparse returns a list of errors and line numbers (and I wanted to keep track of them). At the moment, I'm setting error.message to errors[0].message, but this may change in the future.

I don't know yet if I'm going to stick to papaparse, because it fails to parse many CSV files I've tested (I'm still fiddling with the settings).

The PR is still work in progress. It works, but it still has a few rough corners.

* This connector takes a URL as input, downloads the URL and parses it as a CSV file into a Javascript object that can be queried using alaSQL. * Added test specs and the script `test-unit-csv` to run them. * Updated `Setting.react.js` so that future connectors don't need to update it (updated `NEW_CONNECTION.md` accordingly). * TODO (in future PRs): - `papaparse` parses CSV data only into numbers and strings. Add option to let the user set a date format for parsing. - let the user assign a table name - let the user upload multiple CSV files Closesplotly#349

n-riesco · 2018-02-16T19:09:05Z

@shannonlal I've just fixed the last few rough corners with this PR. I think the PR is ready for review. Would you like to review it?

shannonlal · 2018-02-16T19:59:40Z

@n-riesco Yes I will have time to look this over on the weekend

shannonlal

All the comments I added are small and minor. Everything looks good it was just some minor suggestions. I think it is ready for a dancer. Just let me know if you think my changes make sense or if you want to merge the changes as is.

shannonlal · 2018-02-17T19:02:11Z

+ if (cell) return type(cell);
+ }
+
+ return 'String';


Maybe just add a quick comment to explain that default is string. Or add to JSDocs to function

shannonlal · 2018-02-17T19:03:14Z

+CSVError.prototype.constructor = CSVError;
+
+// the data parsed from CSV files is stored here
+const connectionData = {};


In #362 One of the comments was to start using JSDocs style for our code. Should we be doing this on the backend as well?

shannonlal · 2018-02-17T19:04:16Z

+ connectionData[connection.database] = data;
+}
+
+export function connect(connection) {


If the url is bad, how will this be presented to the user on the Settings UI? will fetch throw an error and this will be gracefully passed to the UI on its own?

Same behaviour for all the connectors, i.e.: fetch throws, routes.js catches the exception and replies to the connection request with a 500 status and the body {error: message: error.message}}.

shannonlal · 2018-02-17T19:05:24Z

+
+export function tables() {
+ // To take advantage of alaSQL's parser, the table is named '?'
+ return Promise.resolve(['?']);


Should we be using '?' or a const CSV_TABLE_NAME = '?'. This way if it changes in the future you don't have to string replace to make sure we find all changes

n-riesco force-pushed the csv-connector branch from 1698f97 to dc00d62Compare February 16, 2018 10:26

shannonlal reviewed Feb 16, 2018
View reviewed changes

n-riesco force-pushed the csv-connector branch from dc00d62 to aecf162Compare February 16, 2018 18:42

n-riesco changed the title ~~[WIP] CSV connector~~CSV connectorFeb 16, 2018

shannonlal reviewed Feb 17, 2018
View reviewed changes

csv: use JSDoc and define constant TABLENAME
92fc235

n-riesco force-pushed the csv-connector branch from 89ddd97 to 92fc235Compare February 19, 2018 15:02

n-riesco merged commit bb661bc into plotly:masterFeb 21, 2018

n-riesco force-pushed the csv-connector branch from 1698f97 to dc00d62Compare February 16, 2018 10:26

shannonlal reviewed Feb 16, 2018
View reviewed changes

n-riesco force-pushed the csv-connector branch from dc00d62 to aecf162Compare February 16, 2018 18:42

n-riesco changed the title ~~[WIP] CSV connector~~CSV connectorFeb 16, 2018

shannonlal reviewed Feb 17, 2018
View reviewed changes

csv: use JSDoc and define constant TABLENAME
92fc235

n-riesco force-pushed the csv-connector branch from 89ddd97 to 92fc235Compare February 19, 2018 15:02

n-riesco merged commit bb661bc into plotly:masterFeb 21, 2018

n-riesco force-pushed the csv-connector branch from 1698f97 to dc00d62Compare February 16, 2018 10:26

shannonlal reviewed Feb 16, 2018
View reviewed changes

n-riesco force-pushed the csv-connector branch from dc00d62 to aecf162Compare February 16, 2018 18:42

n-riesco changed the title ~~[WIP] CSV connector~~CSV connectorFeb 16, 2018

shannonlal reviewed Feb 17, 2018
View reviewed changes

csv: use JSDoc and define constant TABLENAME
92fc235

n-riesco force-pushed the csv-connector branch from 89ddd97 to 92fc235Compare February 19, 2018 15:02

n-riesco merged commit bb661bc into plotly:masterFeb 21, 2018

n-riesco force-pushed the csv-connector branch from 1698f97 to dc00d62Compare February 16, 2018 10:26

shannonlal reviewed Feb 16, 2018
View reviewed changes

n-riesco force-pushed the csv-connector branch from dc00d62 to aecf162Compare February 16, 2018 18:42

n-riesco changed the title ~~[WIP] CSV connector~~CSV connectorFeb 16, 2018

shannonlal reviewed Feb 17, 2018
View reviewed changes

csv: use JSDoc and define constant TABLENAME
92fc235

n-riesco force-pushed the csv-connector branch from 89ddd97 to 92fc235Compare February 19, 2018 15:02

n-riesco merged commit bb661bc into plotly:masterFeb 21, 2018

n-riesco force-pushed the csv-connector branch from 1698f97 to dc00d62Compare February 16, 2018 10:26

shannonlal reviewed Feb 16, 2018
View reviewed changes

n-riesco force-pushed the csv-connector branch from dc00d62 to aecf162Compare February 16, 2018 18:42

n-riesco changed the title ~~[WIP] CSV connector~~CSV connectorFeb 16, 2018

shannonlal reviewed Feb 17, 2018
View reviewed changes

csv: use JSDoc and define constant TABLENAME
92fc235

n-riesco force-pushed the csv-connector branch from 89ddd97 to 92fc235Compare February 19, 2018 15:02

n-riesco merged commit bb661bc into plotly:masterFeb 21, 2018

n-riesco force-pushed the csv-connector branch from 1698f97 to dc00d62Compare February 16, 2018 10:26

shannonlal reviewed Feb 16, 2018
View reviewed changes

n-riesco force-pushed the csv-connector branch from dc00d62 to aecf162Compare February 16, 2018 18:42

n-riesco changed the title ~~[WIP] CSV connector~~CSV connectorFeb 16, 2018

shannonlal reviewed Feb 17, 2018
View reviewed changes

csv: use JSDoc and define constant TABLENAME
92fc235

n-riesco force-pushed the csv-connector branch from 89ddd97 to 92fc235Compare February 19, 2018 15:02

n-riesco merged commit bb661bc into plotly:masterFeb 21, 2018

n-riesco force-pushed the csv-connector branch from 1698f97 to dc00d62Compare February 16, 2018 10:26

shannonlal reviewed Feb 16, 2018
View reviewed changes

n-riesco force-pushed the csv-connector branch from dc00d62 to aecf162Compare February 16, 2018 18:42

n-riesco changed the title ~~[WIP] CSV connector~~CSV connectorFeb 16, 2018

shannonlal reviewed Feb 17, 2018
View reviewed changes

csv: use JSDoc and define constant TABLENAME
92fc235

n-riesco force-pushed the csv-connector branch from 89ddd97 to 92fc235Compare February 19, 2018 15:02

n-riesco merged commit bb661bc into plotly:masterFeb 21, 2018

n-riesco force-pushed the csv-connector branch from 1698f97 to dc00d62Compare February 16, 2018 10:26

shannonlal reviewed Feb 16, 2018
View reviewed changes

n-riesco force-pushed the csv-connector branch from dc00d62 to aecf162Compare February 16, 2018 18:42

n-riesco changed the title ~~[WIP] CSV connector~~CSV connectorFeb 16, 2018

shannonlal reviewed Feb 17, 2018
View reviewed changes

csv: use JSDoc and define constant TABLENAME
92fc235

n-riesco force-pushed the csv-connector branch from 89ddd97 to 92fc235Compare February 19, 2018 15:02

n-riesco merged commit bb661bc into plotly:masterFeb 21, 2018

Uh oh!

Uh oh!

Conversation

n-riesco commented Feb 15, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shannonlal commented Feb 16, 2018

Uh oh!

n-riesco commented Feb 16, 2018

Uh oh!

n-riesco commented Feb 16, 2018

Uh oh!

shannonlal commented Feb 16, 2018

Uh oh!

shannonlal left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

n-riesco commented Feb 15, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shannonlal commented Feb 16, 2018

Uh oh!

n-riesco commented Feb 16, 2018

Uh oh!

n-riesco commented Feb 16, 2018

Uh oh!

shannonlal commented Feb 16, 2018

Uh oh!

shannonlal left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

n-riesco commented Feb 15, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shannonlal commented Feb 16, 2018