-
Notifications
You must be signed in to change notification settings - Fork 14k
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
feat(SIP-39): Websocket sidecar app #11498
Changes from 19 commits
e476ca5
881e18d
01e9f1f
a1954a3
3823bba
c42cbc2
08531f5
1ecf689
dc968bc
63c765f
5eab894
04e8d85
bd273ff
bab9e4b
09506f9
c2c35e5
4976608
dd31aab
f5087ed
8eadc3a
fedf0f3
d44742d
23d17bb
863b503
20b563a
3be73e4
3af092f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
name: WebSocket server | ||
on: | ||
push: | ||
paths: | ||
- "superset-websocket/**" | ||
pull_request: | ||
paths: | ||
- "superset-websocket/**" | ||
|
||
jobs: | ||
app-checks: | ||
if: github.event.pull_request.draft == false | ||
runs-on: ubuntu-20.04 | ||
steps: | ||
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )" | ||
uses: actions/checkout@v2 | ||
with: | ||
persist-credentials: false | ||
- name: Install dependencies | ||
working-directory: ./superset-websocket | ||
run: npm install | ||
- name: lint | ||
working-directory: ./superset-websocket | ||
run: npm run lint | ||
- name: unit tests | ||
working-directory: ./superset-websocket | ||
run: npm run test | ||
- name: build | ||
working-directory: ./superset-websocket | ||
run: npm run build |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
# | ||
# Licensed to the Apache Software Foundation (ASF) under one or more | ||
# contributor license agreements. See the NOTICE file distributed with | ||
# this work for additional information regarding copyright ownership. | ||
# The ASF licenses this file to You under the Apache License, Version 2.0 | ||
# (the "License"); you may not use this file except in compliance with | ||
# the License. You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
# | ||
*.min.js | ||
node_modules | ||
dist | ||
coverage |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
/** | ||
* Licensed to the Apache Software Foundation (ASF) under one | ||
* or more contributor license agreements. See the NOTICE file | ||
* distributed with this work for additional information | ||
* regarding copyright ownership. The ASF licenses this file | ||
* to you under the Apache License, Version 2.0 (the | ||
* "License"); you may not use this file except in compliance | ||
* with the License. You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, | ||
* software distributed under the License is distributed on an | ||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
* KIND, either express or implied. See the License for the | ||
* specific language governing permissions and limitations | ||
* under the License. | ||
*/ | ||
module.exports = { | ||
root: true, | ||
parser: '@typescript-eslint/parser', | ||
env: { | ||
node: true, | ||
browser: true, | ||
}, | ||
plugins: [ | ||
'@typescript-eslint', | ||
], | ||
extends: [ | ||
'eslint:recommended', | ||
'plugin:@typescript-eslint/recommended', | ||
], | ||
rules: { | ||
// "import/no-unresolved": 0, | ||
// "@typescript-eslint/explicit-function-return-type": 0, | ||
"@typescript-eslint/explicit-module-boundary-types": 0, | ||
"@typescript-eslint/no-var-requires": 0, | ||
// "@typescript-eslint/camelcase": 0, | ||
}, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
# | ||
# Licensed to the Apache Software Foundation (ASF) under one or more | ||
# contributor license agreements. See the NOTICE file distributed with | ||
# this work for additional information regarding copyright ownership. | ||
# The ASF licenses this file to You under the Apache License, Version 2.0 | ||
# (the "License"); you may not use this file except in compliance with | ||
# the License. You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
# | ||
config.json | ||
dist |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,80 @@ | ||
<!-- | ||
Licensed to the Apache Software Foundation (ASF) under one | ||
or more contributor license agreements. See the NOTICE file | ||
distributed with this work for additional information | ||
regarding copyright ownership. The ASF licenses this file | ||
to you under the Apache License, Version 2.0 (the | ||
"License"); you may not use this file except in compliance | ||
with the License. You may obtain a copy of the License at | ||
|
||
http://www.apache.org/licenses/LICENSE-2.0 | ||
|
||
Unless required by applicable law or agreed to in writing, | ||
software distributed under the License is distributed on an | ||
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
KIND, either express or implied. See the License for the | ||
specific language governing permissions and limitations | ||
under the License. | ||
--> | ||
# Superset WebSocket Server | ||
|
||
A Node.js WebSocket server for sending async event data to the Superset web application frontend, based on [SIP-39](https://github.com/apache/superset/issues/9190). | ||
|
||
## Requirements | ||
|
||
- Node.js 12+ (not tested with older versions) | ||
- Redis 5+ | ||
|
||
To use this feature, Superset needs to be configured to enable global async queries and to use WebSockets as the transport (see below). | ||
|
||
## Install | ||
|
||
Install dependencies: | ||
``` | ||
npm install | ||
``` | ||
|
||
## WebSocket Server Configuration | ||
|
||
Copy `config.example.json` to `config.json` and adjust the values for your environment. | ||
|
||
## Superset configuration | ||
|
||
Configure the Superset Flask app to enable global async queries (in `superset_config.py`): | ||
|
||
Enable the `GLOBAL_ASYNC_QUERIES` feature flag: | ||
``` | ||
"GLOBAL_ASYNC_QUERIES": True | ||
``` | ||
|
||
Configure the following Superset values: | ||
``` | ||
GLOBAL_ASYNC_QUERIES_TRANSPORT = "ws" | ||
GLOBAL_ASYNC_QUERIES_WEBSOCKET_URL = "ws://<host>:<port>/" | ||
``` | ||
|
||
Note that the WebSocket server must be run on the same hostname (different port) for cookies to be shared between the Flask app and the WebSocket server. | ||
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. Is the JWT the only thing needed from the cookies? If so, have we considered passing that value explicitly? Seems like if we could do that, we could remove this assumption/constraint. 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 could pass it explicitly, though we'd have to make the cookie readable by JS (currently it's httponly), so there is a bit of a tradeoff here. 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. Keeping the cookie httponly is a good move! |
||
|
||
The following config values must contain the same values in both the Flask app config and `config.json`: | ||
``` | ||
GLOBAL_ASYNC_QUERIES_REDIS_CONFIG | ||
GLOBAL_ASYNC_QUERIES_REDIS_STREAM_PREFIX | ||
GLOBAL_ASYNC_QUERIES_JWT_COOKIE_NAME | ||
GLOBAL_ASYNC_QUERIES_JWT_SECRET | ||
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. idea, non-blocking: If these are not configured correctly, can we have the service detect it and provide hints to help devs/admins get it working? |
||
``` | ||
|
||
More info on Superset configuration values for async queries: https://github.com/apache/superset/blob/master/CONTRIBUTING.md#async-chart-queries | ||
|
||
## Running | ||
|
||
Running locally via dev server: | ||
``` | ||
npm run dev-server | ||
``` | ||
|
||
Running remotely: | ||
``` | ||
npm run build && npm start | ||
``` | ||
|
||
*TODO: containerization* |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
{ | ||
"port": 8080, | ||
"redis": { | ||
"port": 6379, | ||
"host": "127.0.0.1", | ||
"password": "", | ||
"db": 0 | ||
}, | ||
"streamPrefix": "async-events-", | ||
"jwtSecret": "CHANGE-ME", | ||
"jwtCookieName": "async-token" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
{ | ||
"redis": { | ||
"port": 6379, | ||
"host": "127.0.0.1", | ||
"password": "", | ||
"db": 10 | ||
}, | ||
"streamPrefix": "test-async-events-", | ||
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. change request: Can we make this more specific? Not clear what a |
||
"jwtSecret": "test123-test123-test123-test123-test123-test123-test123", | ||
"jwtCookieName": "test-async-token" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
/** | ||
* Licensed to the Apache Software Foundation (ASF) under one | ||
* or more contributor license agreements. See the NOTICE file | ||
* distributed with this work for additional information | ||
* regarding copyright ownership. The ASF licenses this file | ||
* to you under the Apache License, Version 2.0 (the | ||
* "License"); you may not use this file except in compliance | ||
* with the License. You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, | ||
* software distributed under the License is distributed on an | ||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
* KIND, either express or implied. See the License for the | ||
* specific language governing permissions and limitations | ||
* under the License. | ||
*/ | ||
module.exports = { | ||
preset: 'ts-jest', | ||
testEnvironment: 'node', | ||
}; |
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.
Should this ignore
node_modules
?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.
node_modules
is ignored in the top-level.gitignore
, but I'll make it explicit.