You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/03/12 18:50:07 UTC
[GitHub] [incubator-superset] ktmud opened a new pull request #9296: Allow
webpack-dev-server proxy to any destination
ktmud opened a new pull request #9296: Allow webpack-dev-server proxy to any destination
URL: https://github.com/apache/incubator-superset/pull/9296
### CATEGORY
- [x] Build / Development Environment
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TEST PLAN
<!--- What steps should be taken to verify the changes -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Changes UI
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
### REVIEWERS
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-io edited a comment on issue #9296:
[DEV] Allow webpack-dev-server proxy to any destination
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #9296: [DEV] Allow webpack-dev-server proxy to any destination
URL: https://github.com/apache/incubator-superset/pull/9296#issuecomment-599146449
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9296?src=pr&el=h1) Report
> Merging [#9296](https://codecov.io/gh/apache/incubator-superset/pull/9296?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/b1916a190e12a4da4b9afaca414ed8bd59a024c8?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9296?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9296 +/- ##
=======================================
Coverage 59.08% 59.08%
=======================================
Files 374 374
Lines 12202 12202
Branches 2986 2986
=======================================
Hits 7209 7209
Misses 4814 4814
Partials 179 179
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9296?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9296?src=pr&el=footer). Last update [b1916a1...19c3422](https://codecov.io/gh/apache/incubator-superset/pull/9296?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] ktmud commented on a change in pull request
#9296: [DEV] Allow webpack-dev-server proxy to any destination
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #9296: [DEV] Allow webpack-dev-server proxy to any destination
URL: https://github.com/apache/incubator-superset/pull/9296#discussion_r392517452
##########
File path: superset-frontend/webpack.config.js
##########
@@ -292,27 +288,50 @@ const config = {
'react/lib/ReactContext': true,
},
plugins,
- devtool: isDevMode ? 'cheap-module-eval-source-map' : false,
- devServer: {
+ devtool: false,
+};
+
+let proxyConfig = {};
+const requireModule = module.require;
+
+function loadProxyConfig() {
+ try {
+ delete require.cache[require.resolve('./webpack.proxy-config')];
+ proxyConfig = requireModule('./webpack.proxy-config');
+ } catch (e) {
+ if (e.code !== 'ENOENT') {
+ console.error('\n>> Error loading proxy config:');
+ console.trace(e);
+ }
+ }
+}
+
+if (isDevMode) {
+ config.devtool = 'cheap-module-eval-source-map';
+
+ config.devServer = {
+ before() {
+ loadProxyConfig();
+ // hot reloading proxy config
+ fs.watch('./webpack.proxy-config.js', loadProxyConfig);
+ },
historyApiFallback: true,
hot: true,
- index: '', // This line is needed to enable root proxying
inline: true,
stats: 'minimal',
overlay: true,
port: devserverPort,
// Only serves bundled files from webpack-dev-server
// and proxy everything else to Superset backend
- proxy: {
- context: () => true,
- '/': `http://localhost:${supersetPort}`,
- target: `http://localhost:${supersetPort}`,
- },
+ proxy: [
+ // functions are called for every request
+ () => {
+ return proxyConfig;
+ },
+ ],
Review comment:
Hot reload borrowed from https://github.com/webpack/webpack-dev-server/tree/master/examples/general/proxy-hot-reload
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] kristw commented on a change in pull request
#9296: [DEV] Allow webpack-dev-server proxy to any destination
Posted by GitBox <gi...@apache.org>.
kristw commented on a change in pull request #9296: [DEV] Allow webpack-dev-server proxy to any destination
URL: https://github.com/apache/incubator-superset/pull/9296#discussion_r393221550
##########
File path: superset-frontend/package.json
##########
@@ -236,7 +236,8 @@
"webpack-bundle-analyzer": "^3.4.1",
"webpack-cli": "^3.1.1",
"webpack-dev-server": "^3.1.14",
- "webpack-sources": "^1.1.0"
+ "webpack-sources": "^1.1.0",
+ "yargs": "12 - 15"
Review comment:
> jest-cli and webpack-dev-server uses 12, vega-lite uses 15. 15 is the latest version, the latest webpack-dev-server hasn't upgraded it yet.
Do those list `yargs` as `peerDependencies` or `dependencies`? `npm` allow nested dependencies so it may install multiple versions of `yargs`, which is inefficient to have multiple copies, but not too bad. IMO, it might be good to be specific for the script stability.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] ktmud commented on a change in pull request
#9296: Allow webpack-dev-server proxy to any destination
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #9296: Allow webpack-dev-server proxy to any destination
URL: https://github.com/apache/incubator-superset/pull/9296#discussion_r392068382
##########
File path: superset-frontend/webpack.proxy-config.js
##########
@@ -0,0 +1,171 @@
+/**
+ * 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.
+ */
+const fs = require('fs');
+const zlib = require('zlib');
+const path = require('path');
+// eslint-disable-next-line
+const parsedArgs = require('minimist')(process.argv.slice(2));
+
+const { supersetPort = 8088, superset: supersetUrl = null } = parsedArgs;
+const backend = (supersetUrl || `http://localhost:${supersetPort}`).replace(
+ '//+$/',
+ '',
+); // strip ending backslash
+const MANIFEST_FILE = path.resolve(
+ __dirname,
+ '../superset/static/assets/manifest.json',
+);
+
+let manifestContent;
+let manifest;
+function loadManifest() {
+ try {
+ const newContent = fs.readFileSync(MANIFEST_FILE);
+ if (!newContent || newContent === manifestContent) return;
+ manifestContent = newContent;
+ manifest = JSON.parse(manifestContent);
+ console.log(`${MANIFEST_FILE} loaded.`);
+ } catch (e) {
+ if (e.code !== 'ENOENT') {
+ console.error('\n>> Error loading manifest file:');
+ console.trace(e);
+ }
+ }
+}
+// make sure the manifest file exists
+fs.mkdirSync(path.dirname(MANIFEST_FILE), { recursive: true });
+fs.closeSync(fs.openSync(MANIFEST_FILE, 'as+'));
+fs.watch(MANIFEST_FILE, loadManifest);
+
+function isHTML(res) {
+ const CONTENT_TYPE_HEADER = 'content-type';
+ const contentType = res.getHeader
+ ? res.getHeader(CONTENT_TYPE_HEADER)
+ : res.headers[CONTENT_TYPE_HEADER];
+ return contentType.includes('text/html');
+}
+
+function toDevHTML(origHtml) {
+ let html = origHtml.replace(
+ /(<head>\s*<title>)([\s\S]*)(<\/title>)/i,
+ '$1[DEV] $2 $3',
+ );
+ // load manifest file only when needed
+ if (!manifest) {
+ loadManifest();
+ }
+ if (manifest) {
+ // replace bundled asset files, HTML comment tags generated by Jinja macros
+ // in superset/templates/superset/partials/asset_bundle.html
+ html = html.replace(
+ /<!-- Bundle (css|js) (.*?) START -->[\s\S]*?<!-- Bundle \1 \2 END -->/gi,
+ (match, assetType, bundleName) => {
+ if (bundleName in manifest.entrypoints) {
+ return `<!-- DEV bundle: ${bundleName} ${assetType} -->${(
+ manifest.entrypoints[bundleName][assetType] || []
+ )
+ .map(chunkFilePath => {
+ if (assetType === 'css') {
+ return `<link rel="stylesheet" type="text/css" href="${chunkFilePath}" />`;
+ }
+ return `<script src="${chunkFilePath}"></script>`;
+ })
+ .join('\n ')}<!-- DEV bundle: ${bundleName} ${assetType} END -->`;
+ }
+ return match;
+ },
+ );
+ }
+ return html;
+}
+
+function copyHeaders(origRes, res) {
+ res.statusCode = origRes.statusCode;
+ res.statusMessage = origRes.statusMessage;
+ if (res.setHeader) {
+ const resIsHTML = isHTML(origRes);
+ for (const [key, value_] of Object.entries(origRes.headers)) {
+ let value = value_;
+ if (resIsHTML) {
+ if (key === 'content-encoding' || key === 'content-length') {
+ continue;
+ }
Review comment:
Keep all original response headers, but skip `content-encoding` and `content-length` for HTMLs because the modified dev HTML will not be gzipped.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-io commented on issue #9296: [DEV]
Allow webpack-dev-server proxy to any destination
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #9296: [DEV] Allow webpack-dev-server proxy to any destination
URL: https://github.com/apache/incubator-superset/pull/9296#issuecomment-599146449
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9296?src=pr&el=h1) Report
> Merging [#9296](https://codecov.io/gh/apache/incubator-superset/pull/9296?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/b1916a190e12a4da4b9afaca414ed8bd59a024c8?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9296?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9296 +/- ##
=======================================
Coverage 59.08% 59.08%
=======================================
Files 374 374
Lines 12202 12202
Branches 2986 2986
=======================================
Hits 7209 7209
Misses 4814 4814
Partials 179 179
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9296?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9296?src=pr&el=footer). Last update [b1916a1...4b7a62e](https://codecov.io/gh/apache/incubator-superset/pull/9296?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] ktmud commented on a change in pull request
#9296: [DEV] Allow webpack-dev-server proxy to any destination
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #9296: [DEV] Allow webpack-dev-server proxy to any destination
URL: https://github.com/apache/incubator-superset/pull/9296#discussion_r392517240
##########
File path: superset-frontend/package.json
##########
@@ -236,7 +236,8 @@
"webpack-bundle-analyzer": "^3.4.1",
"webpack-cli": "^3.1.1",
"webpack-dev-server": "^3.1.14",
- "webpack-sources": "^1.1.0"
+ "webpack-sources": "^1.1.0",
+ "yargs": "12 - 15"
Review comment:
Replace `minimist` with `yargs` as the latter has better API and is included by other packages anyway. Choose this wide range (`12 -15`) also because it is tested working with the latest version but other packages still depend on v12.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] kristw commented on a change in pull request
#9296: [DEV] Allow webpack-dev-server proxy to any destination
Posted by GitBox <gi...@apache.org>.
kristw commented on a change in pull request #9296: [DEV] Allow webpack-dev-server proxy to any destination
URL: https://github.com/apache/incubator-superset/pull/9296#discussion_r392522560
##########
File path: superset-frontend/webpack.proxy-config.js
##########
@@ -0,0 +1,172 @@
+/* eslint-disable no-console */
+/**
+ * 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.
+ */
+const fs = require('fs');
+const zlib = require('zlib');
+const path = require('path');
+// eslint-disable-next-line import/no-extraneous-dependencies
+const parsedArgs = require('yargs').argv;
+
+const { supersetPort = 8088, superset: supersetUrl = null } = parsedArgs;
+const backend = (supersetUrl || `http://localhost:${supersetPort}`).replace(
+ '//+$/',
+ '',
+); // strip ending backslash
+const MANIFEST_FILE = path.resolve(
+ __dirname,
+ '../superset/static/assets/manifest.json',
+);
+
+let manifestContent;
+let manifest;
+function loadManifest() {
+ try {
+ const newContent = fs.readFileSync(MANIFEST_FILE);
+ if (!newContent || newContent === manifestContent) return;
+ manifestContent = newContent;
+ manifest = JSON.parse(manifestContent);
+ console.log(`${MANIFEST_FILE} loaded.`);
+ } catch (e) {
+ if (e.code !== 'ENOENT') {
+ console.error('\n>> Error loading manifest file:');
+ console.trace(e);
+ }
+ }
+}
+// make sure the manifest file exists
+fs.mkdirSync(path.dirname(MANIFEST_FILE), { recursive: true });
+fs.closeSync(fs.openSync(MANIFEST_FILE, 'as+'));
+fs.watch(MANIFEST_FILE, loadManifest);
+
+function isHTML(res) {
+ const CONTENT_TYPE_HEADER = 'content-type';
+ const contentType = res.getHeader
+ ? res.getHeader(CONTENT_TYPE_HEADER)
+ : res.headers[CONTENT_TYPE_HEADER];
+ return contentType.includes('text/html');
+}
+
+function toDevHTML(origHtml) {
+ let html = origHtml.replace(
+ /(<head>\s*<title>)([\s\S]*)(<\/title>)/i,
+ '$1[DEV] $2 $3',
+ );
+ // load manifest file only when needed
+ if (!manifest) {
+ loadManifest();
+ }
+ if (manifest) {
+ // replace bundled asset files, HTML comment tags generated by Jinja macros
+ // in superset/templates/superset/partials/asset_bundle.html
+ html = html.replace(
+ /<!-- Bundle (css|js) (.*?) START -->[\s\S]*?<!-- Bundle \1 \2 END -->/gi,
+ (match, assetType, bundleName) => {
+ if (bundleName in manifest.entrypoints) {
+ return `<!-- DEV bundle: ${bundleName} ${assetType} -->${(
+ manifest.entrypoints[bundleName][assetType] || []
+ )
+ .map(chunkFilePath => {
+ if (assetType === 'css') {
+ return `<link rel="stylesheet" type="text/css" href="${chunkFilePath}" />`;
+ }
+ return `<script src="${chunkFilePath}"></script>`;
+ })
+ .join('\n ')}<!-- DEV bundle: ${bundleName} ${assetType} END -->`;
+ }
+ return match;
+ },
+ );
+ }
+ return html;
+}
+
+function copyHeaders(origRes, res) {
+ res.statusCode = origRes.statusCode;
+ res.statusMessage = origRes.statusMessage;
+ if (res.setHeader) {
+ const resIsHTML = isHTML(origRes);
+ for (const [key, value_] of Object.entries(origRes.headers)) {
+ let value = value_;
+ if (resIsHTML) {
+ if (key === 'content-encoding' || key === 'content-length') {
+ continue;
+ }
+ }
+ if (key === 'set-cookie') {
+ // remove cookie domain
+ value = Array.isArray(value) ? value : [value];
+ value = value.map(x => x.replace(/Domain=[^;]+?/i, ''));
+ } else if (key === 'location') {
+ // set redirects to use local URL
+ value = (value || '').replace(backend, '');
+ }
+ res.setHeader(key, value);
+ }
+ } else {
+ res.headers = origRes.headers;
+ }
+}
+
+/**
+ * Manipulate HTML server response to replace asset files with
+ * local webpack-dev-server build.
+ */
+function processHTML(proxyRes, res) {
+ let body = Buffer.from([]);
+ let origRes = proxyRes;
+
+ // decode GZIP response
+ if (origRes.headers['content-encoding'] === 'gzip') {
+ const gunzip = zlib.createGunzip();
+ origRes.pipe(gunzip);
+ origRes = gunzip;
+ }
+
+ origRes.on('data', function(data) {
+ body = Buffer.concat([body, data]);
+ });
+ origRes.on('end', function() {
+ body = body.toString();
+ res.end(toDevHTML(body));
Review comment:
perhaps possible to chain?
```js
origRes
.on('data', data => { body = Buffer.concat([body, data]); })
.on('end', () => { res.end(toDevHTML(body.toString())); });
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] kristw commented on a change in pull request
#9296: [DEV] Allow webpack-dev-server proxy to any destination
Posted by GitBox <gi...@apache.org>.
kristw commented on a change in pull request #9296: [DEV] Allow webpack-dev-server proxy to any destination
URL: https://github.com/apache/incubator-superset/pull/9296#discussion_r393221894
##########
File path: superset-frontend/webpack.proxy-config.js
##########
@@ -0,0 +1,172 @@
+/* eslint-disable no-console */
+/**
+ * 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.
+ */
+const fs = require('fs');
+const zlib = require('zlib');
+const path = require('path');
+// eslint-disable-next-line import/no-extraneous-dependencies
+const parsedArgs = require('yargs').argv;
+
+const { supersetPort = 8088, superset: supersetUrl = null } = parsedArgs;
+const backend = (supersetUrl || `http://localhost:${supersetPort}`).replace(
+ '//+$/',
+ '',
+); // strip ending backslash
+const MANIFEST_FILE = path.resolve(
+ __dirname,
+ '../superset/static/assets/manifest.json',
+);
+
+let manifestContent;
+let manifest;
+function loadManifest() {
+ try {
+ const newContent = fs.readFileSync(MANIFEST_FILE);
+ if (!newContent || newContent === manifestContent) return;
+ manifestContent = newContent;
+ manifest = JSON.parse(manifestContent);
+ console.log(`${MANIFEST_FILE} loaded.`);
+ } catch (e) {
+ if (e.code !== 'ENOENT') {
+ console.error('\n>> Error loading manifest file:');
+ console.trace(e);
+ }
+ }
+}
+// make sure the manifest file exists
+fs.mkdirSync(path.dirname(MANIFEST_FILE), { recursive: true });
+fs.closeSync(fs.openSync(MANIFEST_FILE, 'as+'));
+fs.watch(MANIFEST_FILE, loadManifest);
+
+function isHTML(res) {
+ const CONTENT_TYPE_HEADER = 'content-type';
+ const contentType = res.getHeader
+ ? res.getHeader(CONTENT_TYPE_HEADER)
+ : res.headers[CONTENT_TYPE_HEADER];
+ return contentType.includes('text/html');
+}
+
+function toDevHTML(origHtml) {
+ let html = origHtml.replace(
Review comment:
That would be nice.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] ktmud commented on a change in pull request
#9296: Allow webpack-dev-server proxy to any destination
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #9296: Allow webpack-dev-server proxy to any destination
URL: https://github.com/apache/incubator-superset/pull/9296#discussion_r392070164
##########
File path: superset/__init__.py
##########
@@ -43,7 +43,7 @@
cache = LocalProxy(lambda: cache_manager.cache)
conf = LocalProxy(lambda: current_app.config)
get_feature_flags = feature_flag_manager.get_feature_flags
-get_css_manifest_files = manifest_processor.get_css_manifest_files
+get_manifest_files = manifest_processor.get_manifest_files
Review comment:
This is breaking change. Anyone using `import get_css_manifest_files from superset` should take note.
But I suspect it's not used anywhere except `viz.py` (code search on GitHub found no other references).
In the future, Superset modules should probably use `__all__` to specify what to export publicly, and internal modules should not `import ... from superset` root.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] ktmud commented on issue #9296: [DEV] Allow
webpack-dev-server proxy to any destination
Posted by GitBox <gi...@apache.org>.
ktmud commented on issue #9296: [DEV] Allow webpack-dev-server proxy to any destination
URL: https://github.com/apache/incubator-superset/pull/9296#issuecomment-600334271
Tested again with a fresh installation. Didn't see any issues. I think this is ready to merge.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] ktmud commented on a change in pull request
#9296: Allow webpack-dev-server proxy to any destination
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #9296: Allow webpack-dev-server proxy to any destination
URL: https://github.com/apache/incubator-superset/pull/9296#discussion_r392072067
##########
File path: superset/templates/superset/partials/asset_bundle.html
##########
@@ -16,8 +16,20 @@
specific language governing permissions and limitations
under the License.
#}
-{% block partial_js %}
- {% for entry in get_unloaded_chunks(js_manifest(filename), loaded_chunks) %}
+{% macro js_bundle(filename) %}
+ {# HTML comment is needed for webpack-dev-server to replace assets
+ with development version #}
+ <!-- Bundle js {{ filename }} START -->
Review comment:
The downside is this adds such HTML comments even to the production mode. Can probably create a config flag `DISABLE_ASSET_BUNDLE_WATERMARK` to turn this off.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] kristw commented on a change in pull request
#9296: [DEV] Allow webpack-dev-server proxy to any destination
Posted by GitBox <gi...@apache.org>.
kristw commented on a change in pull request #9296: [DEV] Allow webpack-dev-server proxy to any destination
URL: https://github.com/apache/incubator-superset/pull/9296#discussion_r392541424
##########
File path: superset-frontend/webpack.proxy-config.js
##########
@@ -0,0 +1,171 @@
+/**
+ * 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.
+ */
+const fs = require('fs');
+const zlib = require('zlib');
+const path = require('path');
+// eslint-disable-next-line
+const parsedArgs = require('minimist')(process.argv.slice(2));
+
+const { supersetPort = 8088, superset: supersetUrl = null } = parsedArgs;
+const backend = (supersetUrl || `http://localhost:${supersetPort}`).replace(
+ '//+$/',
+ '',
+); // strip ending backslash
+const MANIFEST_FILE = path.resolve(
+ __dirname,
+ '../superset/static/assets/manifest.json',
+);
+
+let manifestContent;
+let manifest;
+function loadManifest() {
+ try {
+ const newContent = fs.readFileSync(MANIFEST_FILE);
+ if (!newContent || newContent === manifestContent) return;
+ manifestContent = newContent;
+ manifest = JSON.parse(manifestContent);
+ console.log(`${MANIFEST_FILE} loaded.`);
+ } catch (e) {
+ if (e.code !== 'ENOENT') {
+ console.error('\n>> Error loading manifest file:');
+ console.trace(e);
+ }
+ }
+}
+// make sure the manifest file exists
+fs.mkdirSync(path.dirname(MANIFEST_FILE), { recursive: true });
+fs.closeSync(fs.openSync(MANIFEST_FILE, 'as+'));
+fs.watch(MANIFEST_FILE, loadManifest);
+
+function isHTML(res) {
+ const CONTENT_TYPE_HEADER = 'content-type';
+ const contentType = res.getHeader
+ ? res.getHeader(CONTENT_TYPE_HEADER)
+ : res.headers[CONTENT_TYPE_HEADER];
+ return contentType.includes('text/html');
+}
+
+function toDevHTML(origHtml) {
+ let html = origHtml.replace(
+ /(<head>\s*<title>)([\s\S]*)(<\/title>)/i,
+ '$1[DEV] $2 $3',
+ );
+ // load manifest file only when needed
+ if (!manifest) {
+ loadManifest();
+ }
+ if (manifest) {
+ // replace bundled asset files, HTML comment tags generated by Jinja macros
+ // in superset/templates/superset/partials/asset_bundle.html
+ html = html.replace(
+ /<!-- Bundle (css|js) (.*?) START -->[\s\S]*?<!-- Bundle \1 \2 END -->/gi,
+ (match, assetType, bundleName) => {
+ if (bundleName in manifest.entrypoints) {
+ return `<!-- DEV bundle: ${bundleName} ${assetType} -->${(
+ manifest.entrypoints[bundleName][assetType] || []
+ )
+ .map(chunkFilePath => {
+ if (assetType === 'css') {
+ return `<link rel="stylesheet" type="text/css" href="${chunkFilePath}" />`;
+ }
+ return `<script src="${chunkFilePath}"></script>`;
+ })
+ .join('\n ')}<!-- DEV bundle: ${bundleName} ${assetType} END -->`;
+ }
+ return match;
+ },
+ );
+ }
+ return html;
+}
+
+function copyHeaders(origRes, res) {
+ res.statusCode = origRes.statusCode;
+ res.statusMessage = origRes.statusMessage;
+ if (res.setHeader) {
+ const resIsHTML = isHTML(origRes);
+ for (const [key, value_] of Object.entries(origRes.headers)) {
+ let value = value_;
+ if (resIsHTML) {
+ if (key === 'content-encoding' || key === 'content-length') {
+ continue;
+ }
Review comment:
I like chaining 😄
```js
Object.keys(origRes.header)
.filter(key => !resIsHTML || (key !== 'content-encoding' && key !== 'content-length'))
.map(key => {
const value = origRes.headers[key];
if(key === 'set-cookie'){
// remove cookie domain
return { key, value: (Array.isArray(value) ? value : [value]).map(x => x.replace(/Domain=[^;]+?/i, '')) };
} else if (key === 'location') {
// set redirects to use local URL
return { key, value: (value || '').replace(backend, '') };
}
return { key, value };
})
.forEach(({ key, value }) => { res.setHeader(key, value); });
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] ktmud commented on a change in pull request
#9296: [DEV] Allow webpack-dev-server proxy to any destination
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #9296: [DEV] Allow webpack-dev-server proxy to any destination
URL: https://github.com/apache/incubator-superset/pull/9296#discussion_r393279713
##########
File path: superset-frontend/package.json
##########
@@ -236,7 +236,8 @@
"webpack-bundle-analyzer": "^3.4.1",
"webpack-cli": "^3.1.1",
"webpack-dev-server": "^3.1.14",
- "webpack-sources": "^1.1.0"
+ "webpack-sources": "^1.1.0",
+ "yargs": "12 - 15"
Review comment:
They list it as `dependencies`. The range is specifically to avoid multiple copies. I think it's OK to keep this range for now as we didn't use it too extensively and we already know both v12 and v15 works for our code.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] ktmud commented on a change in pull request
#9296: [DEV] Allow webpack-dev-server proxy to any destination
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #9296: [DEV] Allow webpack-dev-server proxy to any destination
URL: https://github.com/apache/incubator-superset/pull/9296#discussion_r392517452
##########
File path: superset-frontend/webpack.config.js
##########
@@ -292,27 +288,50 @@ const config = {
'react/lib/ReactContext': true,
},
plugins,
- devtool: isDevMode ? 'cheap-module-eval-source-map' : false,
- devServer: {
+ devtool: false,
+};
+
+let proxyConfig = {};
+const requireModule = module.require;
+
+function loadProxyConfig() {
+ try {
+ delete require.cache[require.resolve('./webpack.proxy-config')];
+ proxyConfig = requireModule('./webpack.proxy-config');
+ } catch (e) {
+ if (e.code !== 'ENOENT') {
+ console.error('\n>> Error loading proxy config:');
+ console.trace(e);
+ }
+ }
+}
+
+if (isDevMode) {
+ config.devtool = 'cheap-module-eval-source-map';
+
+ config.devServer = {
+ before() {
+ loadProxyConfig();
+ // hot reloading proxy config
+ fs.watch('./webpack.proxy-config.js', loadProxyConfig);
+ },
historyApiFallback: true,
hot: true,
- index: '', // This line is needed to enable root proxying
inline: true,
stats: 'minimal',
overlay: true,
port: devserverPort,
// Only serves bundled files from webpack-dev-server
// and proxy everything else to Superset backend
- proxy: {
- context: () => true,
- '/': `http://localhost:${supersetPort}`,
- target: `http://localhost:${supersetPort}`,
- },
+ proxy: [
+ // functions are called for every request
+ () => {
+ return proxyConfig;
+ },
+ ],
Review comment:
Hot reload borrowed from [here](https://github.com/webpack/webpack-dev-server/tree/75718b7e25a27da598340cb00e23628d9496cd1a/examples/general/proxy-hot-reload).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] ktmud commented on a change in pull request
#9296: [DEV] Allow webpack-dev-server proxy to any destination
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #9296: [DEV] Allow webpack-dev-server proxy to any destination
URL: https://github.com/apache/incubator-superset/pull/9296#discussion_r392626993
##########
File path: superset-frontend/webpack.proxy-config.js
##########
@@ -0,0 +1,172 @@
+/* eslint-disable no-console */
+/**
+ * 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.
+ */
+const fs = require('fs');
+const zlib = require('zlib');
+const path = require('path');
+// eslint-disable-next-line import/no-extraneous-dependencies
+const parsedArgs = require('yargs').argv;
+
+const { supersetPort = 8088, superset: supersetUrl = null } = parsedArgs;
+const backend = (supersetUrl || `http://localhost:${supersetPort}`).replace(
+ '//+$/',
+ '',
+); // strip ending backslash
+const MANIFEST_FILE = path.resolve(
+ __dirname,
+ '../superset/static/assets/manifest.json',
+);
+
+let manifestContent;
+let manifest;
+function loadManifest() {
+ try {
+ const newContent = fs.readFileSync(MANIFEST_FILE);
+ if (!newContent || newContent === manifestContent) return;
+ manifestContent = newContent;
+ manifest = JSON.parse(manifestContent);
+ console.log(`${MANIFEST_FILE} loaded.`);
+ } catch (e) {
+ if (e.code !== 'ENOENT') {
+ console.error('\n>> Error loading manifest file:');
+ console.trace(e);
+ }
+ }
+}
+// make sure the manifest file exists
+fs.mkdirSync(path.dirname(MANIFEST_FILE), { recursive: true });
+fs.closeSync(fs.openSync(MANIFEST_FILE, 'as+'));
+fs.watch(MANIFEST_FILE, loadManifest);
+
+function isHTML(res) {
+ const CONTENT_TYPE_HEADER = 'content-type';
+ const contentType = res.getHeader
+ ? res.getHeader(CONTENT_TYPE_HEADER)
+ : res.headers[CONTENT_TYPE_HEADER];
+ return contentType.includes('text/html');
+}
+
+function toDevHTML(origHtml) {
+ let html = origHtml.replace(
Review comment:
I think `origHtml` is more in line with `origRes` used elsewhere in this file, otherwise should I rename `origRes` to `originalResponse`, henceforth `res` to `response`, and `req` to `request` as well?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] kristw commented on a change in pull request
#9296: [DEV] Allow webpack-dev-server proxy to any destination
Posted by GitBox <gi...@apache.org>.
kristw commented on a change in pull request #9296: [DEV] Allow webpack-dev-server proxy to any destination
URL: https://github.com/apache/incubator-superset/pull/9296#discussion_r393226312
##########
File path: superset-frontend/package.json
##########
@@ -236,7 +236,8 @@
"webpack-bundle-analyzer": "^3.4.1",
"webpack-cli": "^3.1.1",
"webpack-dev-server": "^3.1.14",
- "webpack-sources": "^1.1.0"
+ "webpack-sources": "^1.1.0",
+ "yargs": "12 - 15"
Review comment:
Renovate looks interesting.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] kristw commented on a change in pull request
#9296: [DEV] Allow webpack-dev-server proxy to any destination
Posted by GitBox <gi...@apache.org>.
kristw commented on a change in pull request #9296: [DEV] Allow webpack-dev-server proxy to any destination
URL: https://github.com/apache/incubator-superset/pull/9296#discussion_r392525680
##########
File path: superset-frontend/package.json
##########
@@ -236,7 +236,8 @@
"webpack-bundle-analyzer": "^3.4.1",
"webpack-cli": "^3.1.1",
"webpack-dev-server": "^3.1.14",
- "webpack-sources": "^1.1.0"
+ "webpack-sources": "^1.1.0",
+ "yargs": "12 - 15"
Review comment:
What other packages are depending on it? Do we have any script that depends on it?
Could we try to get this to `15`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] kristw commented on a change in pull request
#9296: [DEV] Allow webpack-dev-server proxy to any destination
Posted by GitBox <gi...@apache.org>.
kristw commented on a change in pull request #9296: [DEV] Allow webpack-dev-server proxy to any destination
URL: https://github.com/apache/incubator-superset/pull/9296#discussion_r392524613
##########
File path: superset/__init__.py
##########
@@ -43,7 +43,7 @@
cache = LocalProxy(lambda: cache_manager.cache)
conf = LocalProxy(lambda: current_app.config)
get_feature_flags = feature_flag_manager.get_feature_flags
-get_css_manifest_files = manifest_processor.get_css_manifest_files
+get_manifest_files = manifest_processor.get_manifest_files
Review comment:
This should be ok
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] kristw commented on a change in pull request
#9296: [DEV] Allow webpack-dev-server proxy to any destination
Posted by GitBox <gi...@apache.org>.
kristw commented on a change in pull request #9296: [DEV] Allow webpack-dev-server proxy to any destination
URL: https://github.com/apache/incubator-superset/pull/9296#discussion_r392546539
##########
File path: superset-frontend/webpack.proxy-config.js
##########
@@ -0,0 +1,172 @@
+/* eslint-disable no-console */
+/**
+ * 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.
+ */
+const fs = require('fs');
+const zlib = require('zlib');
+const path = require('path');
+// eslint-disable-next-line import/no-extraneous-dependencies
+const parsedArgs = require('yargs').argv;
+
+const { supersetPort = 8088, superset: supersetUrl = null } = parsedArgs;
+const backend = (supersetUrl || `http://localhost:${supersetPort}`).replace(
+ '//+$/',
+ '',
+); // strip ending backslash
+const MANIFEST_FILE = path.resolve(
+ __dirname,
+ '../superset/static/assets/manifest.json',
+);
+
+let manifestContent;
+let manifest;
+function loadManifest() {
+ try {
+ const newContent = fs.readFileSync(MANIFEST_FILE);
+ if (!newContent || newContent === manifestContent) return;
+ manifestContent = newContent;
+ manifest = JSON.parse(manifestContent);
+ console.log(`${MANIFEST_FILE} loaded.`);
+ } catch (e) {
+ if (e.code !== 'ENOENT') {
+ console.error('\n>> Error loading manifest file:');
+ console.trace(e);
+ }
+ }
+}
+// make sure the manifest file exists
+fs.mkdirSync(path.dirname(MANIFEST_FILE), { recursive: true });
+fs.closeSync(fs.openSync(MANIFEST_FILE, 'as+'));
+fs.watch(MANIFEST_FILE, loadManifest);
+
+function isHTML(res) {
+ const CONTENT_TYPE_HEADER = 'content-type';
+ const contentType = res.getHeader
+ ? res.getHeader(CONTENT_TYPE_HEADER)
+ : res.headers[CONTENT_TYPE_HEADER];
+ return contentType.includes('text/html');
+}
+
+function toDevHTML(origHtml) {
+ let html = origHtml.replace(
Review comment:
* could move this block of code below the `loadManifest()` block.
* can use full word for clarity `origHtml` => `originalHtml`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] ktmud commented on a change in pull request
#9296: Allow webpack-dev-server proxy to any destination
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #9296: Allow webpack-dev-server proxy to any destination
URL: https://github.com/apache/incubator-superset/pull/9296#discussion_r392067835
##########
File path: superset-frontend/webpack.proxy-config.js
##########
@@ -0,0 +1,171 @@
+/**
+ * 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.
+ */
+const fs = require('fs');
+const zlib = require('zlib');
+const path = require('path');
+// eslint-disable-next-line
+const parsedArgs = require('minimist')(process.argv.slice(2));
+
+const { supersetPort = 8088, superset: supersetUrl = null } = parsedArgs;
+const backend = (supersetUrl || `http://localhost:${supersetPort}`).replace(
+ '//+$/',
+ '',
+); // strip ending backslash
+const MANIFEST_FILE = path.resolve(
+ __dirname,
+ '../superset/static/assets/manifest.json',
+);
+
+let manifestContent;
+let manifest;
+function loadManifest() {
+ try {
+ const newContent = fs.readFileSync(MANIFEST_FILE);
+ if (!newContent || newContent === manifestContent) return;
+ manifestContent = newContent;
+ manifest = JSON.parse(manifestContent);
+ console.log(`${MANIFEST_FILE} loaded.`);
+ } catch (e) {
+ if (e.code !== 'ENOENT') {
+ console.error('\n>> Error loading manifest file:');
+ console.trace(e);
+ }
+ }
+}
+// make sure the manifest file exists
+fs.mkdirSync(path.dirname(MANIFEST_FILE), { recursive: true });
+fs.closeSync(fs.openSync(MANIFEST_FILE, 'as+'));
+fs.watch(MANIFEST_FILE, loadManifest);
Review comment:
Reload manifest every time there is a new build.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] ktmud commented on a change in pull request
#9296: [DEV] Allow webpack-dev-server proxy to any destination
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #9296: [DEV] Allow webpack-dev-server proxy to any destination
URL: https://github.com/apache/incubator-superset/pull/9296#discussion_r392552126
##########
File path: superset-frontend/webpack.proxy-config.js
##########
@@ -0,0 +1,171 @@
+/**
+ * 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.
+ */
+const fs = require('fs');
+const zlib = require('zlib');
+const path = require('path');
+// eslint-disable-next-line
+const parsedArgs = require('minimist')(process.argv.slice(2));
+
+const { supersetPort = 8088, superset: supersetUrl = null } = parsedArgs;
+const backend = (supersetUrl || `http://localhost:${supersetPort}`).replace(
+ '//+$/',
+ '',
+); // strip ending backslash
+const MANIFEST_FILE = path.resolve(
+ __dirname,
+ '../superset/static/assets/manifest.json',
+);
+
+let manifestContent;
+let manifest;
+function loadManifest() {
+ try {
+ const newContent = fs.readFileSync(MANIFEST_FILE);
+ if (!newContent || newContent === manifestContent) return;
+ manifestContent = newContent;
+ manifest = JSON.parse(manifestContent);
+ console.log(`${MANIFEST_FILE} loaded.`);
+ } catch (e) {
+ if (e.code !== 'ENOENT') {
+ console.error('\n>> Error loading manifest file:');
+ console.trace(e);
+ }
+ }
+}
+// make sure the manifest file exists
+fs.mkdirSync(path.dirname(MANIFEST_FILE), { recursive: true });
+fs.closeSync(fs.openSync(MANIFEST_FILE, 'as+'));
+fs.watch(MANIFEST_FILE, loadManifest);
+
+function isHTML(res) {
+ const CONTENT_TYPE_HEADER = 'content-type';
+ const contentType = res.getHeader
+ ? res.getHeader(CONTENT_TYPE_HEADER)
+ : res.headers[CONTENT_TYPE_HEADER];
+ return contentType.includes('text/html');
+}
+
+function toDevHTML(origHtml) {
+ let html = origHtml.replace(
+ /(<head>\s*<title>)([\s\S]*)(<\/title>)/i,
+ '$1[DEV] $2 $3',
+ );
+ // load manifest file only when needed
+ if (!manifest) {
+ loadManifest();
+ }
+ if (manifest) {
+ // replace bundled asset files, HTML comment tags generated by Jinja macros
+ // in superset/templates/superset/partials/asset_bundle.html
+ html = html.replace(
+ /<!-- Bundle (css|js) (.*?) START -->[\s\S]*?<!-- Bundle \1 \2 END -->/gi,
+ (match, assetType, bundleName) => {
+ if (bundleName in manifest.entrypoints) {
+ return `<!-- DEV bundle: ${bundleName} ${assetType} -->${(
+ manifest.entrypoints[bundleName][assetType] || []
+ )
+ .map(chunkFilePath => {
+ if (assetType === 'css') {
+ return `<link rel="stylesheet" type="text/css" href="${chunkFilePath}" />`;
+ }
+ return `<script src="${chunkFilePath}"></script>`;
+ })
+ .join('\n ')}<!-- DEV bundle: ${bundleName} ${assetType} END -->`;
+ }
+ return match;
+ },
+ );
+ }
+ return html;
+}
+
+function copyHeaders(origRes, res) {
+ res.statusCode = origRes.statusCode;
+ res.statusMessage = origRes.statusMessage;
+ if (res.setHeader) {
+ const resIsHTML = isHTML(origRes);
+ for (const [key, value_] of Object.entries(origRes.headers)) {
+ let value = value_;
+ if (resIsHTML) {
+ if (key === 'content-encoding' || key === 'content-length') {
+ continue;
+ }
Review comment:
I'm not sure about this one... This is definitely more concise, but it's also three loops instead of one. Plus I liked the explicitness and directness of the naive for loops and continue/breaks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] ktmud commented on a change in pull request
#9296: [DEV] Allow webpack-dev-server proxy to any destination
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #9296: [DEV] Allow webpack-dev-server proxy to any destination
URL: https://github.com/apache/incubator-superset/pull/9296#discussion_r393330791
##########
File path: superset-frontend/webpack.proxy-config.js
##########
@@ -0,0 +1,172 @@
+/* eslint-disable no-console */
+/**
+ * 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.
+ */
+const fs = require('fs');
+const zlib = require('zlib');
+const path = require('path');
+// eslint-disable-next-line import/no-extraneous-dependencies
+const parsedArgs = require('yargs').argv;
+
+const { supersetPort = 8088, superset: supersetUrl = null } = parsedArgs;
+const backend = (supersetUrl || `http://localhost:${supersetPort}`).replace(
+ '//+$/',
+ '',
+); // strip ending backslash
+const MANIFEST_FILE = path.resolve(
+ __dirname,
+ '../superset/static/assets/manifest.json',
+);
+
+let manifestContent;
+let manifest;
+function loadManifest() {
+ try {
+ const newContent = fs.readFileSync(MANIFEST_FILE);
+ if (!newContent || newContent === manifestContent) return;
+ manifestContent = newContent;
+ manifest = JSON.parse(manifestContent);
+ console.log(`${MANIFEST_FILE} loaded.`);
+ } catch (e) {
+ if (e.code !== 'ENOENT') {
+ console.error('\n>> Error loading manifest file:');
+ console.trace(e);
+ }
+ }
+}
+// make sure the manifest file exists
+fs.mkdirSync(path.dirname(MANIFEST_FILE), { recursive: true });
+fs.closeSync(fs.openSync(MANIFEST_FILE, 'as+'));
+fs.watch(MANIFEST_FILE, loadManifest);
+
+function isHTML(res) {
+ const CONTENT_TYPE_HEADER = 'content-type';
+ const contentType = res.getHeader
+ ? res.getHeader(CONTENT_TYPE_HEADER)
+ : res.headers[CONTENT_TYPE_HEADER];
+ return contentType.includes('text/html');
+}
+
+function toDevHTML(origHtml) {
+ let html = origHtml.replace(
Review comment:
Updated the variable names!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] ktmud commented on a change in pull request
#9296: [DEV] Allow webpack-dev-server proxy to any destination
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #9296: [DEV] Allow webpack-dev-server proxy to any destination
URL: https://github.com/apache/incubator-superset/pull/9296#discussion_r392627226
##########
File path: superset-frontend/webpack.proxy-config.js
##########
@@ -0,0 +1,171 @@
+/**
+ * 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.
+ */
+const fs = require('fs');
+const zlib = require('zlib');
+const path = require('path');
+// eslint-disable-next-line
+const parsedArgs = require('minimist')(process.argv.slice(2));
+
+const { supersetPort = 8088, superset: supersetUrl = null } = parsedArgs;
+const backend = (supersetUrl || `http://localhost:${supersetPort}`).replace(
+ '//+$/',
+ '',
+); // strip ending backslash
+const MANIFEST_FILE = path.resolve(
+ __dirname,
+ '../superset/static/assets/manifest.json',
+);
+
+let manifestContent;
+let manifest;
+function loadManifest() {
+ try {
+ const newContent = fs.readFileSync(MANIFEST_FILE);
+ if (!newContent || newContent === manifestContent) return;
+ manifestContent = newContent;
+ manifest = JSON.parse(manifestContent);
+ console.log(`${MANIFEST_FILE} loaded.`);
+ } catch (e) {
+ if (e.code !== 'ENOENT') {
+ console.error('\n>> Error loading manifest file:');
+ console.trace(e);
+ }
+ }
+}
+// make sure the manifest file exists
+fs.mkdirSync(path.dirname(MANIFEST_FILE), { recursive: true });
+fs.closeSync(fs.openSync(MANIFEST_FILE, 'as+'));
+fs.watch(MANIFEST_FILE, loadManifest);
+
+function isHTML(res) {
+ const CONTENT_TYPE_HEADER = 'content-type';
+ const contentType = res.getHeader
+ ? res.getHeader(CONTENT_TYPE_HEADER)
+ : res.headers[CONTENT_TYPE_HEADER];
+ return contentType.includes('text/html');
+}
+
+function toDevHTML(origHtml) {
+ let html = origHtml.replace(
+ /(<head>\s*<title>)([\s\S]*)(<\/title>)/i,
+ '$1[DEV] $2 $3',
+ );
+ // load manifest file only when needed
+ if (!manifest) {
+ loadManifest();
+ }
+ if (manifest) {
+ // replace bundled asset files, HTML comment tags generated by Jinja macros
+ // in superset/templates/superset/partials/asset_bundle.html
+ html = html.replace(
+ /<!-- Bundle (css|js) (.*?) START -->[\s\S]*?<!-- Bundle \1 \2 END -->/gi,
+ (match, assetType, bundleName) => {
+ if (bundleName in manifest.entrypoints) {
+ return `<!-- DEV bundle: ${bundleName} ${assetType} -->${(
+ manifest.entrypoints[bundleName][assetType] || []
+ )
+ .map(chunkFilePath => {
+ if (assetType === 'css') {
+ return `<link rel="stylesheet" type="text/css" href="${chunkFilePath}" />`;
+ }
+ return `<script src="${chunkFilePath}"></script>`;
+ })
+ .join('\n ')}<!-- DEV bundle: ${bundleName} ${assetType} END -->`;
+ }
+ return match;
+ },
+ );
+ }
+ return html;
+}
+
+function copyHeaders(origRes, res) {
+ res.statusCode = origRes.statusCode;
+ res.statusMessage = origRes.statusMessage;
+ if (res.setHeader) {
+ const resIsHTML = isHTML(origRes);
+ for (const [key, value_] of Object.entries(origRes.headers)) {
+ let value = value_;
+ if (resIsHTML) {
+ if (key === 'content-encoding' || key === 'content-length') {
+ continue;
+ }
Review comment:
```js
Object.keys(origRes.headers)
.filter(
key =>
!resIsHTML || key !== 'content-encoding' || key !== 'content-length',
)
.forEach(key => {
let value = origRes.headers[key];
if (key === 'set-cookie') {
// remove cookie domain
value = Array.isArray(value) ? value : [value];
value = value.map(x => x.replace(/Domain=[^;]+?/i, ''));
} else if (key === 'location') {
// set redirects to use local URL
value = (value || '').replace(backend, '');
}
res.setHeader(key, value);
});
```
This is probably good enough.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-io edited a comment on issue #9296:
[DEV] Allow webpack-dev-server proxy to any destination
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #9296: [DEV] Allow webpack-dev-server proxy to any destination
URL: https://github.com/apache/incubator-superset/pull/9296#issuecomment-599146449
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9296?src=pr&el=h1) Report
> Merging [#9296](https://codecov.io/gh/apache/incubator-superset/pull/9296?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/b1916a190e12a4da4b9afaca414ed8bd59a024c8?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9296?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9296 +/- ##
=======================================
Coverage 59.08% 59.08%
=======================================
Files 374 374
Lines 12202 12202
Branches 2986 2986
=======================================
Hits 7209 7209
Misses 4814 4814
Partials 179 179
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9296?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9296?src=pr&el=footer). Last update [b1916a1...53ed851](https://codecov.io/gh/apache/incubator-superset/pull/9296?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-io edited a comment on issue #9296:
[DEV] Allow webpack-dev-server proxy to any destination
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #9296: [DEV] Allow webpack-dev-server proxy to any destination
URL: https://github.com/apache/incubator-superset/pull/9296#issuecomment-599146449
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9296?src=pr&el=h1) Report
> Merging [#9296](https://codecov.io/gh/apache/incubator-superset/pull/9296?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/b1916a190e12a4da4b9afaca414ed8bd59a024c8?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9296?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9296 +/- ##
=======================================
Coverage 59.08% 59.08%
=======================================
Files 374 374
Lines 12202 12202
Branches 2986 2986
=======================================
Hits 7209 7209
Misses 4814 4814
Partials 179 179
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9296?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9296?src=pr&el=footer). Last update [b1916a1...bc1d07c](https://codecov.io/gh/apache/incubator-superset/pull/9296?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] ktmud commented on a change in pull request
#9296: [DEV] Allow webpack-dev-server proxy to any destination
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #9296: [DEV] Allow webpack-dev-server proxy to any destination
URL: https://github.com/apache/incubator-superset/pull/9296#discussion_r392551270
##########
File path: superset-frontend/package.json
##########
@@ -236,7 +236,8 @@
"webpack-bundle-analyzer": "^3.4.1",
"webpack-cli": "^3.1.1",
"webpack-dev-server": "^3.1.14",
- "webpack-sources": "^1.1.0"
+ "webpack-sources": "^1.1.0",
+ "yargs": "12 - 15"
Review comment:
`jest-cli` and `webpack-dev-server` uses `12`, `vega-lite` uses `15`. `15` is the latest version, the latest `webpack-dev-server` hasn't upgraded it yet.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] kristw merged pull request #9296: [DEV] Allow
webpack-dev-server proxy to any destination
Posted by GitBox <gi...@apache.org>.
kristw merged pull request #9296: [DEV] Allow webpack-dev-server proxy to any destination
URL: https://github.com/apache/incubator-superset/pull/9296
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] kristw commented on a change in pull request
#9296: [DEV] Allow webpack-dev-server proxy to any destination
Posted by GitBox <gi...@apache.org>.
kristw commented on a change in pull request #9296: [DEV] Allow webpack-dev-server proxy to any destination
URL: https://github.com/apache/incubator-superset/pull/9296#discussion_r392521646
##########
File path: superset-frontend/webpack.proxy-config.js
##########
@@ -0,0 +1,172 @@
+/* eslint-disable no-console */
+/**
+ * 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.
+ */
+const fs = require('fs');
+const zlib = require('zlib');
+const path = require('path');
+// eslint-disable-next-line import/no-extraneous-dependencies
+const parsedArgs = require('yargs').argv;
+
+const { supersetPort = 8088, superset: supersetUrl = null } = parsedArgs;
+const backend = (supersetUrl || `http://localhost:${supersetPort}`).replace(
+ '//+$/',
+ '',
+); // strip ending backslash
+const MANIFEST_FILE = path.resolve(
+ __dirname,
+ '../superset/static/assets/manifest.json',
+);
+
+let manifestContent;
+let manifest;
+function loadManifest() {
+ try {
+ const newContent = fs.readFileSync(MANIFEST_FILE);
+ if (!newContent || newContent === manifestContent) return;
+ manifestContent = newContent;
+ manifest = JSON.parse(manifestContent);
+ console.log(`${MANIFEST_FILE} loaded.`);
+ } catch (e) {
+ if (e.code !== 'ENOENT') {
+ console.error('\n>> Error loading manifest file:');
+ console.trace(e);
+ }
+ }
+}
+// make sure the manifest file exists
+fs.mkdirSync(path.dirname(MANIFEST_FILE), { recursive: true });
+fs.closeSync(fs.openSync(MANIFEST_FILE, 'as+'));
+fs.watch(MANIFEST_FILE, loadManifest);
+
+function isHTML(res) {
+ const CONTENT_TYPE_HEADER = 'content-type';
+ const contentType = res.getHeader
+ ? res.getHeader(CONTENT_TYPE_HEADER)
+ : res.headers[CONTENT_TYPE_HEADER];
+ return contentType.includes('text/html');
+}
+
+function toDevHTML(origHtml) {
+ let html = origHtml.replace(
+ /(<head>\s*<title>)([\s\S]*)(<\/title>)/i,
+ '$1[DEV] $2 $3',
+ );
+ // load manifest file only when needed
+ if (!manifest) {
+ loadManifest();
+ }
+ if (manifest) {
+ // replace bundled asset files, HTML comment tags generated by Jinja macros
+ // in superset/templates/superset/partials/asset_bundle.html
+ html = html.replace(
+ /<!-- Bundle (css|js) (.*?) START -->[\s\S]*?<!-- Bundle \1 \2 END -->/gi,
+ (match, assetType, bundleName) => {
+ if (bundleName in manifest.entrypoints) {
+ return `<!-- DEV bundle: ${bundleName} ${assetType} -->${(
+ manifest.entrypoints[bundleName][assetType] || []
+ )
+ .map(chunkFilePath => {
Review comment:
```js
.map(chunkFilePath => (assetType === 'css')
? `<link rel="stylesheet" type="text/css" href="${chunkFilePath}" />`
: `<script src="${chunkFilePath}"></script>`);
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] ktmud commented on a change in pull request
#9296: [DEV] Allow webpack-dev-server proxy to any destination
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #9296: [DEV] Allow webpack-dev-server proxy to any destination
URL: https://github.com/apache/incubator-superset/pull/9296#discussion_r392551775
##########
File path: superset-frontend/package.json
##########
@@ -236,7 +236,8 @@
"webpack-bundle-analyzer": "^3.4.1",
"webpack-cli": "^3.1.1",
"webpack-dev-server": "^3.1.14",
- "webpack-sources": "^1.1.0"
+ "webpack-sources": "^1.1.0",
+ "yargs": "12 - 15"
Review comment:
Wow, [just found out](https://github.com/webpack/webpack-dev-server/pull/2318) there is [a bot ](https://github.com/marketplace/renovate)that automatically upgrade the packages for you, plus migrating all the code. Maybe we can use it to replace dependabot in `superset-ui` and `superset-ui-plugins`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org