-
Notifications
You must be signed in to change notification settings - Fork 97
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
FLAG-1114: create datamart api route #4810
Conversation
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.
🚀
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.
Summary
Nice work, @wri7tno ! 💪
It would be great if we could push more of the query param building, parsing, and payload transformations into the server side of Flagship.
Specifically, I'm wondering if we can move the parts that I have highlighted in the red boxes.
What's Implemented in this PR
---
title: Implemented in this PR
---
sequenceDiagram
box rgba(0,125,0,0.1) [GFW Flagship] <br/> Client Side
actor widgetRenderer as widgetRenderer?
participant widget as Widget <br/> [net-change/index.js]
participant serviceAnalysis as Service <br/> [analysis-cached.js]
end
box orange [GFW Flagship] <br/> Server Side
participant dataMart as pages/api <br/> [datamart/net-change/index.js]
end
box gray GFW Data API
participant gfwDataApi as GFW Data API
end
widgetRenderer->>widget: getData()
rect rgba(125,0,0,0.5)
widget->>widget: set admin values <br/> to null if 'global'
end
widget->>serviceAnalysis: getNetChange
rect rgba(125,0,0,0.5)
serviceAnalysis->>serviceAnalysis: add 'version' <br/> and 'fields' <br/> params
note over serviceAnalysis: 'version' and 'fields' are hard-coded
end
serviceAnalysis->>dataMart: [GET] api/datamart/net-change
activate dataMart
note over dataMart: [All in one method] <br/> 1. Assert correct GADM admin values <br/> 2. Build the query
alt is download
dataMart-->>serviceAnalysis: 200 OK
note over serviceAnalysis,dataMart: { data: { url: /dataset.../download/csv?sql=... } }
rect rgba(125,0,0,0.5)
serviceAnalysis->>serviceAnalysis: create indicator object
end
serviceAnalysis-->>widget: { name: net_tree_cover_change.... , url: <gfwDataApi url> }
widget-->>widgetRenderer: wrap result in an array: []
else
dataMart->>gfwDataApi: [GET] legacy query endpoint
gfwDataApi-->>dataMart: 200 OK
dataMart-->>serviceAnalysis: 200 OK
note over serviceAnalysis,dataMart: { data: { ...actual queried data results... } }
serviceAnalysis-->>widget: { data: { data: <dataMart response> } }
rect rgba(125,0,0,0.5)
note over widget: Transform the response again. <br/> Add widget settings and options.
end
widget-->>widgetRenderer:
end
deactivate dataMart
My Suggestion to Move Even More Functionality
I tried to represent the functionality represented in the red boxes (above) being moved into the server side (see diagram below).
---
title: Suggested Changes
---
sequenceDiagram
box rgba(0,125,0,0.1) Client Side
actor widgetRenderer as widgetRenderer?
participant widget as Widget <br/> [net-change/index.js]
participant serviceAnalysis as Service <br/> [analysis-cached.js]
end
box orange Server Side
participant dataMart as pages/api <br/> [datamart/net-change/index.js]
end
box gray GFW Data API
participant gfwDataApi as GFW Data API
end
widgetRenderer->>widget: getData()
widget->>serviceAnalysis: getNetChange
serviceAnalysis->>dataMart: [GET] api/datamart/net-change
activate dataMart
dataMart->>dataMart: set admin values <br/> with a value object
note over dataMart: Value object automatically asserts correct usage
dataMart->>dataMart: add 'version' <br/> and 'fields' <br/> params
note over dataMart: 'version' and 'fields' are hard-coded
dataMart->>dataMart: Build the query
alt is Download
dataMart->>dataMart: create indicator object
dataMart-->>serviceAnalysis: 200 OK
note over serviceAnalysis,dataMart: { name: net_tree_cover_change.... , url: <gfwDataApi url> }
serviceAnalysis-->>widget:
widget-->>widgetRenderer: wrap result in an array: []
else
dataMart->>gfwDataApi: [GET] legacy query endpoint
gfwDataApi-->>dataMart: 200 OK
dataMart->>dataMart: Transform the response again. <br/> Add start/end year and range info.
dataMart->>serviceAnalysis: 200 OK
note over serviceAnalysis,dataMart: { data: { ...actual queried data results... , ...range info...} }
serviceAnalysis-->>widget:
widget->>widget: Minor transform to add settings and options <br/> [specific to widget styling]
widget-->>widgetRenderer:
end
deactivate dataMart
This would remove even more data massaging responsibilities from the client.
|
||
url = `${url}/${dataset}/${version}/${ | ||
isDownload ? 'download/csv' : 'query' | ||
}?sql=SELECT ${fieldsList} FROM data ${where}`; |
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.
Then, you could replace the explicit sql
param value with a function that calls the helper functions mentioned above.
Overview
Create a new api route that serves as a proxy to hide all SQL details from client side, this PoC will serve as the first step to implement a data mart on the API side by the engineering team.
The new api route only works for the net change widget since it is a simple one that doesn't have dynamic filters.
Original client request:
New client request:
Demo
Notes
If applicable: ancilary topics, caveats, alternative strategies that didn't work out, etc.
Testing