-
-
Notifications
You must be signed in to change notification settings - Fork 281
Implement API to create a grid to store results of a scheduled query #444
Changes from 4 commits
26ec584
c26fa9d
b3504bf
3aa1c34
728888b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,17 +1,32 @@ | ||
import * as Connections from './datastores/Datastores'; | ||
import {getConnectionById} from './Connections'; | ||
import {getQuery, getQueries, saveQuery, deleteQuery} from './Queries'; | ||
import {getSetting} from '../settings'; | ||
import Logger from '../logger'; | ||
import {PlotlyAPIRequest, updateGrid} from './PlotlyAPI'; | ||
import {has} from 'ramda'; | ||
|
||
import {getConnectionById} from './Connections.js'; | ||
import * as Connections from './datastores/Datastores.js'; | ||
import Logger from '../logger'; | ||
import { | ||
getQuery, | ||
getQueries, | ||
saveQuery, | ||
deleteQuery | ||
} from './Queries.js'; | ||
import { | ||
getCredentials, | ||
getSetting | ||
} from '../settings.js'; | ||
import { | ||
getCurrentUser, | ||
getGridMetadata, | ||
newGrid, | ||
updateGrid | ||
} from './plotly-api.js'; | ||
|
||
class QueryScheduler { | ||
constructor() { | ||
this.scheduleQuery = this.scheduleQuery.bind(this); | ||
this.loadQueries = this.loadQueries.bind(this); | ||
this.clearQuery = this.clearQuery.bind(this); | ||
this.clearQueries = this.clearQueries.bind(this); | ||
this.queryAndCreateGrid = this.queryAndCreateGrid.bind(this); | ||
this.queryAndUpdateGrid = this.queryAndUpdateGrid.bind(this); | ||
|
||
// this.job wraps this.queryAndUpdateGrid to avoid concurrent runs of the same job | ||
|
@@ -106,7 +121,69 @@ class QueryScheduler { | |
Object.keys(this.queryJobs).forEach(this.clearQuery); | ||
} | ||
|
||
queryAndUpdateGrid (fid, uids, queryString, connectionId, requestor) { | ||
queryAndCreateGrid(filename, query, connectionId, requestor) { | ||
const {username, apiKey, accessToken} = getCredentials(requestor); | ||
let startTime; | ||
|
||
// Check if the user even exists | ||
if (!username || !(apiKey || accessToken)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can simplify it:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. tricky, but not equivalent: the latter doesn't catch the case when |
||
/* | ||
* Warning: The front end looks for "Unauthenticated" in this error message. Don't change it! | ||
*/ | ||
const errorMessage = ( | ||
'Unauthenticated: Attempting to create a grid but the ' + | ||
`authentication credentials for the user "${username}" do not exist.` | ||
); | ||
Logger.log(errorMessage, 0); | ||
throw new Error(errorMessage); | ||
} | ||
|
||
// Check if the credentials are valid | ||
return getCurrentUser(username).then(res => { | ||
if (res.status !== 200) { | ||
const errorMessage = ( | ||
`Unauthenticated: ${getSetting('PLOTLY_API_URL')} failed to identify ${username}.` | ||
); | ||
Logger.log(errorMessage, 0); | ||
throw new Error(errorMessage); | ||
} | ||
|
||
|
||
startTime = process.hrtime(); | ||
|
||
Logger.log(`Querying "${query}" with connection ${connectionId} to create a new grid`, 2); | ||
return Connections.query(query, getConnectionById(connectionId)); | ||
|
||
}).then(({rows, columnnames}) => { | ||
Logger.log(`Query "${query}" took ${process.hrtime(startTime)[0]} seconds`, 2); | ||
Logger.log('Create a new grid with new data', 2); | ||
Logger.log(`First row: ${JSON.stringify(rows.slice(0, 1))}`, 2); | ||
|
||
startTime = process.hrtime(); | ||
|
||
return newGrid( | ||
filename, | ||
columnnames, | ||
rows, | ||
requestor | ||
); | ||
|
||
}).then(res => { | ||
Logger.log(`Request to Plotly for creating a grid took ${process.hrtime(startTime)[0]} seconds`, 2); | ||
|
||
if (res.status !== 201) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should be putting this into a constants file with actual codes? I noticed below we have 200, 401, etc. If we used names would make code easier to read There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, it'd make the code less readable (in this context it's clear 201 is an HTTP status code). |
||
Logger.log(`Error ${res.status} while creating a grid`, 2); | ||
} | ||
|
||
return res.json().then((json) => { | ||
Logger.log(`Grid ${json.file.fid} has been updated.`, 2); | ||
return json; | ||
}); | ||
}); | ||
|
||
} | ||
|
||
queryAndUpdateGrid(fid, uids, query, connectionId, requestor) { | ||
const requestedDBConnections = getConnectionById(connectionId); | ||
let startTime = process.hrtime(); | ||
|
||
|
@@ -118,16 +195,12 @@ class QueryScheduler { | |
* If the user is the owner, then requestor === fid.split(':')[0] | ||
* If the user is a collaborator, then requestor is different | ||
*/ | ||
const username = requestor; | ||
const user = getSetting('USERS').find( | ||
u => u.username === username | ||
); | ||
const {username, apiKey, accessToken} = getCredentials(requestor); | ||
|
||
// Check if the user even exists | ||
if (!user || !(user.apiKey || user.accessToken)) { | ||
if (!username || !(apiKey || accessToken)) { | ||
/* | ||
* Heads up - the front end looks for "Unauthenticated" in this | ||
* error message. So don't change it! | ||
* Warning: The front end looks for "Unauthenticated" in this error message. Don't change it! | ||
*/ | ||
const errorMessage = ( | ||
`Unauthenticated: Attempting to update grid ${fid} but the ` + | ||
|
@@ -136,11 +209,9 @@ class QueryScheduler { | |
Logger.log(errorMessage, 0); | ||
throw new Error(errorMessage); | ||
} | ||
const {apiKey, accessToken} = user; | ||
|
||
// Check if the credentials are valid | ||
return PlotlyAPIRequest('users/current', { | ||
method: 'GET', username, apiKey, accessToken | ||
}).then(res => { | ||
return getCurrentUser(username).then(res => { | ||
if (res.status !== 200) { | ||
const errorMessage = ( | ||
`Unauthenticated: ${getSetting('PLOTLY_API_URL')} failed to identify ${username}.` | ||
|
@@ -149,21 +220,22 @@ class QueryScheduler { | |
throw new Error(errorMessage); | ||
} | ||
|
||
Logger.log(`Querying "${queryString}" with connection ${connectionId} to update grid ${fid}`, 2); | ||
return Connections.query(queryString, requestedDBConnections); | ||
Logger.log(`Querying "${query}" with connection ${connectionId} to update grid ${fid}`, 2); | ||
return Connections.query(query, requestedDBConnections); | ||
|
||
}).then(rowsAndColumns => { | ||
}).then(({rows}) => { | ||
|
||
Logger.log(`Query "${queryString}" took ${process.hrtime(startTime)[0]} seconds`, 2); | ||
Logger.log(`Query "${query}" took ${process.hrtime(startTime)[0]} seconds`, 2); | ||
Logger.log(`Updating grid ${fid} with new data`, 2); | ||
Logger.log( | ||
'First row: ' + | ||
JSON.stringify(rowsAndColumns.rows.slice(0, 1)), | ||
JSON.stringify(rows.slice(0, 1)), | ||
2); | ||
|
||
startTime = process.hrtime(); | ||
|
||
return updateGrid( | ||
rowsAndColumns.rows, | ||
rows, | ||
fid, | ||
uids, | ||
requestor | ||
|
@@ -196,16 +268,9 @@ class QueryScheduler { | |
* make an additional API call to GET it | ||
*/ | ||
|
||
return PlotlyAPIRequest( | ||
`grids/${fid}`, | ||
{username, apiKey, accessToken, method: 'GET'} | ||
).then(resFromGET => { | ||
return getGridMetadata(fid, username).then(resFromGET => { | ||
if (resFromGET.status === 404) { | ||
Logger.log(('' + | ||
`Grid ID ${fid} doesn't exist on Plotly anymore, ` + | ||
'removing persistent query.'), | ||
2 | ||
); | ||
Logger.log(`Grid ID ${fid} doesn't exist on Plotly anymore, removing persistent query.`, 2); | ||
this.clearQuery(fid); | ||
return deleteQuery(fid); | ||
} | ||
|
@@ -216,7 +281,6 @@ class QueryScheduler { | |
try { | ||
filemeta = JSON.parse(text); | ||
} catch (e) { | ||
// Well, that's annoying. | ||
Logger.log(`Failed to parse the JSON of request ${fid}`, 0); | ||
Logger.log(e); | ||
Logger.log('Text response: ' + text, 0); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Not necessarily in this PR) but we should small-case these filenames as well.. !!
Queries
,Datastore
etc, and remove.js
extensions to be consistent.. !!There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to keep the extensions, because:
code-editor.css
andcode-editor.jsx
)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea is to move to lower case, whenever we have a chance (instead of having a huge big PR). We don't really want to spend time on it.