[WIP] upload scheduled query metadata

[briandennis]

closes #503

TODO:

• figure out why chart studio still shows incorrect refresh interval despite being set correctly (as far as I can tell)
• research issues related to metadata request failing after updating query content and look into rolling back if needed

[nicolaskruchten]

Process note: could you please target the 3.0-onprem branch instead of master for stuff related to the release? We'll merge that branch into master when it goes gold :)

[n-riesco]

out of curiosity, why setting the default to 'null'?

[n-riesco]

this throws when cronInterval is null

[n-riesco]

@briandennis Fixing the issue with mapCronToRefresh fixes the issue Chart Studio not showing the grid metadata:

diff --git a/backend/utils/cronUtils.js b/backend/utils/cronUtils.js
index f9ce325..c896996 100644
--- a/backend/utils/cronUtils.js
+++ b/backend/utils/cronUtils.js
@@ -23,14 +23,16 @@ export function mapRefreshToCron (refreshInterval) {
 }
 
 export function mapCronToRefresh (cronInterval) {
-    if (cronInterval === '* * * * *') {
-        return 60;
-    } else if (cronInterval === '*/5 * * * *') {
-        return 60 * 5;
-    } else if (cronInterval.match(/\S+? \* \* \* \*/)) {
-        return 60 * 60;
-    } else if (cronInterval.match(/\S+? \S+? \* \* \*/)) {
-        return 60 * 60 * 24;
+    if (cronInterval) {
+        if (cronInterval === '* * * * *') {
+            return 60;
+        } else if (cronInterval === '*/5 * * * *') {
+            return 60 * 5;
+        } else if (cronInterval.match(/\S+? \* \* \* \*/)) {
+            return 60 * 60;
+        } else if (cronInterval.match(/\S+? \S+? \* \* \*/)) {
+            return 60 * 60 * 24;
+        }
     }
 
     // default to weekly
@@ -47,4 +49,4 @@ function computeMinutes (now) {
     }
 
     return minutes.join(',');
-}
\ No newline at end of file
+}

---

While testing sometimes I get:

When this happens, I restart Falcon and indeed the query has been deleted (as one would expect when the associated grid has been deleted).

[n-riesco]

@briandennis And this fixes the issue that was causing refreshInterval to be always set to weekly.

diff --git a/backend/persistent/QueryScheduler.js b/backend/persistent/QueryScheduler.js
index 63375db..139db44 100644
--- a/backend/persistent/QueryScheduler.js
+++ b/backend/persistent/QueryScheduler.js
@@ -23,8 +23,6 @@ import {
     updateGrid
 } from './plotly-api.js';
 
-const DEFAULT_REFRESH_INTERVAL = 60 * 60 * 24 * 7;
-
 class QueryScheduler {
     constructor() {
         this.scheduleQuery = this.scheduleQuery.bind(this);
@@ -100,7 +98,7 @@ class QueryScheduler {
             requestor,
             fid,
             uids,
-            refreshInterval: refreshInterval || DEFAULT_REFRESH_INTERVAL,
+            refreshInterval,
             cronInterval,
             query,
             connectionId

[n-riesco]

@briandennis Please, ignore my comment about the random errors caused by deleted grids. I was using a query left behind by the unit tests.

[nicolaskruchten]

note that the connectorUrl that gets pushed into metadata shouldn't be https://default:9494 as in the screenshot above ... it should be something more like "connectorUrl": "https://nicolas-026480bf-62bf-4b19-be6c-3.plotly-connector.com:9495",

[n-riesco]

@nicolaskruchten please, ignore that screenshot, it was generated using a queries.yaml left behind by the unit tests.

[n-riesco]

This is how it looks like with a query scheduled from Falcon:

[nicolaskruchten]

@briandennis do you need any help getting this guy over the finish line?

[briandennis]

@nicolaskruchten apologies for the delayed fixes here, was traveling these past few days. I believe the only remaining work is looking into whether or not the metadata request failure leaves the grid in a recoverable state. Investigating that now...

[n-riesco]

👍

[briandennis]

After testing with a backend that purposely fails when updating metadata, here's what I've found:

Creating a new scheduled query (queryAndCreate)

• a new grid is created with the query content but with no metadata
• the query is not scheduled or persisted in Falcon
• after a successful retry, a new grid will be tied to the query and the detached grid from the failed attempt will remain
• rollback operation required: delete the detached grid

Updating an existing scheduled query (queryAndUpdate)

• the existing grid is updated with the new content (potentially out of sync with the original query)
• old metadata remains intact
• original scheduled query will continue to run on it's original schedule
• after a successful retry, everything will be updated correctly and the state would be the same as if the failure never happened
• rollback operation required: load the grid's original content before starting the update and repopulate the grid with it after detecting a metadata upload failure

Some things to consider regarding a rollback solution:

• metadata failure should be a very exceptional case (bad query/connection parameters would cause query execution to fail first, bad authentication creds would cause the grid content request to fail first)
• if the problem is network related, the rollback requests would likely still fail
• in both cases, the failure is recoverable (subsequent successful requests resolve inconsistencies and the only side effect would be the addition of the detached grid in the case of create)

@n-riesco @nicolaskruchten let me know how you want to proceed 🙂

[nicolaskruchten]

Generally, I think I would be OK with just letting the user know that something odd had happened with the metadata and that they should hit save again, instead of trying to roll things back etc.

In case 1 there is no metadata, which means that the user can't edit the query from the webapp (meh, the Falcon UI is better) and that the grid isn't indexed as being 'live' (meh, not great but not the end of the world).

In case 2 there is metadata which is wrong, so a user editing the query from the webapp would have an incorrect starting point. This is not great, but again we're going to be discouraging this pattern.

In either case, informing the user and getting them to re-save is good enough I feel, given how rare we expect this situation to be.