[SIP-14] remove dependency on yarn in favor of npm #6541

mistercrunch · 2018-12-16T17:47:53Z

Find more details here #6217

improved the CI caching along the way
simplified a few things in .travis.yml and tox.ini
removed redundant npm run test (npm run cover already does that)
fixed all vulnerabilities identified by npm
used depcheck to find and remove unused deps

Find more details here apache#6217

codecov-io · 2018-12-16T21:55:38Z

Codecov Report

Merging #6541 into master will decrease coverage by 17.46%.
The diff coverage is 88.46%.

@@             Coverage Diff             @@
##           master    #6541       +/-   ##
===========================================
- Coverage    73.5%   56.03%   -17.47%     
===========================================
  Files          72      516      +444     
  Lines       10002    22989    +12987     
  Branches        0     2819     +2819     
===========================================
+ Hits         7352    12882     +5530     
- Misses       2650     9646     +6996     
- Partials        0      461      +461

Impacted Files	Coverage Δ
.../src/dashboard/components/menu/WithPopoverMenu.jsx	`77.55% <100%> (ø)`
...sets/src/dashboard/components/DashboardBuilder.jsx	`79.16% <100%> (ø)`
.../src/dashboard/components/gridComponents/Chart.jsx	`69.56% <100%> (ø)`
...s/src/dashboard/components/gridComponents/Tabs.jsx	`67.27% <100%> (ø)`
...set/assets/src/dashboard/components/SliceAdder.jsx	`80.88% <100%> (ø)`
.../assets/src/dashboard/components/DashboardGrid.jsx	`61.11% <100%> (ø)`
...src/dashboard/components/HeaderActionsDropdown.jsx	`70% <50%> (ø)`
...uperset/assets/src/dashboard/components/Header.jsx	`41.11% <75%> (ø)`
superset/sql_lab.py	`71.26% <0%> (ø)`	⬆️
superset/config.py	`93.46% <0%> (ø)`	⬆️
... and 448 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fe99490...79fc45c. Read the comment docs.

victornoel · 2018-12-17T12:22:03Z

.travis.yml

      before_install:
        - cd superset/assets
      install:
-        - yarn install --frozen-lockfile
+        - npm install


I think the proper command would be npm ci (see https://docs.npmjs.com/cli/ci.html) which is the equivalent of yarn's frozen-lockfile)

victornoel · 2018-12-17T12:22:45Z

contrib/docker/Dockerfile

-    && yarn run build \
-    && rm -rf node_modules \
-    && yarn cache clean
+    && npm install


here too npm ci should be preferred I believe for reproducible builds.

victornoel · 2018-12-17T12:23:29Z

contrib/docker/docker-entrypoint.sh

@@ -6,8 +6,8 @@ if [ "$#" -ne 0 ]; then
 elif [ "$SUPERSET_ENV" = "development" ]; then
    superset worker &
    # needed by superset runserver
-    (cd superset/assets/ && yarn && yarn run sync-backend)
-    (cd superset/assets/ && yarn run dev) &
+    (cd superset/assets/ && npm install && npm run sync-backend)


here it is correct to have npm install I think since this is for dev :)

victornoel · 2018-12-17T12:24:15Z

superset/assets/js_build.sh

@@ -3,8 +3,7 @@ set -e
 cd "$(dirname "$0")"
 npm --version
 node --version
-npm install -g yarn
-yarn
+npm install


I'm not sure about this one… depends on when js_build.sh is used :)

mistercrunch · 2018-12-18T01:37:01Z

Good call. Didn't know about this command, been out off using only yarn for too long now. I altered the PR to use npm ci in all places where it should be used.

phoenix24 · 2018-12-18T05:06:00Z

Build failed, because it couldn't find npm ci.

Here's what i think, npm ci is available in a recent release v6.50, whereas currently used npm version is 5.6.0 by travis. I think you might need to bump up nvm install 8.9 in .travis.yaml

Or probably, you could just use npm clean-install. Hopefully someone could confirm that.

tox.ini

phoenix24 · 2018-12-18T07:00:15Z

Yay! worked.

phoenix24

looks good!

superset/assets/package.json

kristw · 2018-12-18T20:06:45Z

superset/assets/package.json

@@ -67,6 +67,7 @@
    "bootstrap-slider": "^10.0.0",
    "brace": "^0.11.1",
    "classnames": "^2.2.5",
+    "core-js": "^2.6.0",


Where is this one used?

Without core-js, I get build failures like:

ERROR in ./node_modules/@superset-ui/chart/node_modules/@superset-ui/core/esm/models/Registry.js Module not found: Error: Can't resolve 'core-js/modules/es6.array.iterator' in '/home/travis/build/mistercrunch/superset/superset/assets/node_modules/@superset-ui/chart/node_modules/@superset-ui/core/esm/models' @ ./node_modules/@superset-ui/chart/node_modules/@superset-ui/core/esm/models/Registry.js 1:125-168 @ ./node_modules/@superset-ui/chart/node_modules/@superset-ui/core/esm/index.js @ ./node_modules/@superset-ui/chart/esm/registries/ChartTransformPropsRegistrySingleton.js @ ./node_modules/@superset-ui/chart/esm/index.js @ ./src/addSlice/AddSliceContainer.jsx @ ./src/addSlice/App.jsx @ ./src/addSlice/index.jsx @ multi babel-polyfill ./src/preamble.js ./src/addSlice/index.jsx

core-js appears to be polyfill-related, I'm unclear if it's a build issue with superset-ui, some babel issue or something else, but adding core-js fixes it.

It should be imported via babel-polyfill. Probably need to update @superset-ui/core config to list that as dependency.

Fixed all issues by upgrading to babel 7.*

mistercrunch · 2018-12-19T04:02:36Z

Just used depcheck to find and remove unused deps

mistercrunch · 2018-12-21T04:37:14Z

@betodealmeida mind taking a look?

betodealmeida · 2018-12-21T18:24:45Z

Awesome, happy to see this getting some love! :)

* [SIP-14] remove dependency on yarn in favor of npm Find more details here apache#6217 * Add core-js to dev deps * lint * fix cypress * break down cypress_build.sh * fix cypress * Use 'npm ci' in place of 'npm install' * Bump npm req to >=6.5.0 * Enforcing npm@'>=6.5.0' * Using latest lts of nvm * Make cache settings global * remove uneeded 'npm run test' as 'npm run cover' takes care of that * Prefix with 'time' * Trying to upgrade babel-eslint instead of downgrading * upgrading babel-polyfill to '@babel/polyfill' * Moving to babel 7 * remove unused packages

* [SIP-3] Scheduled email reports for Slices / Dashboards (#5294) * [scheduled reports] Add support for scheduled reports * Scheduled email reports for slice and dashboard visualization (attachment or inline) * Scheduled email reports for slice data (CSV attachment on inline table) * Each schedule has a list of recipients (all of them can receive a single mail, or separate mails) * All outgoing mails can have a mandatory bcc - for audit purposes. * Each dashboard/slice can have multiple schedules. In addition, this PR also makes a few minor improvements to the celery infrastructure. * Create a common celery app * Added more celery annotations for the tasks * Introduced celery beat * Update docs about concurrency / pools * [scheduled reports] - Debug mode for scheduled emails * [scheduled reports] - Ability to send test mails * [scheduled reports] - Test email functionality - minor improvements * [scheduled reports] - Rebase with master. Minor fixes * [scheduled reports] - Add warning messages * [scheduled reports] - flake8 * [scheduled reports] - fix rebase * [scheduled reports] - fix rebase * [scheduled reports] - fix flake8 * [scheduled reports] Rebase in prep for merge * Fixed alembic tree after rebase * Updated requirements to latest version of packages (and tested) * Removed py2 stuff * [scheduled reports] - fix flake8 * [scheduled reports] - address review comments * [scheduled reports] - rebase with master * Typo: Fixed link (#6087) * Update contributing.md with latest local dev instructions (#6513) * Fix malformed table in docs/visualization.rst (#6409) * Update requests version (#6510) * Update requests version * Run pip-compile --output-file requirements.txt * [warm] Enforcing consistent form-data (#6531) * Avoid resetting margin to 0 (#6536) * Avoid resetting margin to 0 * Fixing margin for pie chart * Refactor teradata to new time_grain_functions spec (#6539) * Refactor teradata to new time_grain_functions spec * Add test for time_grain_functions * Add docker files to gitignore (#6507) * add docker files to gitignore * Update .gitignore * adding in a dependency version to fix an error with Flask CLI (#6547) * Fix string value displaying NaN (#6534) * Documentation Correction to use http.server for Python3 (#6549) * Remove note about snowflake-sqlalchemy stable version regression (#6398) * filter_values documentation fix (#5977) * Pass security manager to QUERY_LOGGER (#6548) * Pass security manager to log_query * Fix lint * Increase size of column `name` in table `ab_view_meu` (#6532) * Create migration script * Use batch operation for ALTER COLUMN * fix addr_str format bug (#6551) * Fix deck.gl Polygon not show (#6545) * Fix deck.gl Polygon not show * Replace "//" to "// " * Secure unsecured views and prevent regressions (#6553) * Secure views and prevent regressions * Force POST on shortner * Fix tests * [cosmetic] add css no-wrap on CRUD's last modified (#6522) * [RfC] Fix URL too long (#6519) When a URL gets too long (usually happens with controls that allow free form text), it creates an array of bugs. * bug when creating a short URL, because the POST request's referrer field is too long * a bug when saving a chart (when the modal shows up) also because of the referrer being too long Some work has been done in the past to mitigate this, and I'm unclear if it's some sort of regression because of the Flask upgrade or some browser change, or whether these bugs have always been around. This is a request for comment on an approach that works. Let me know if you can think of better ideas as to how to manage this. My current solution looks for 8000+ characters URLs and shrinks them to only `/superset/explore/?URL_IS_TOO_LONG_TO_SHARE&form_data=...` and we only keep the formData keys for `datasource` and `viz_type`. Not super elegant but does the trick. * minor, auto tune debug mode when use flask_env (#6550) * minor, auto tune debug mode when use flask_env * update changes * Make owner a m2m relation on datasources (#6544) * Make owner a m2m relation on datasources * Fix pylint * Make migration work in mysql & sqlite * Fix multiple db_migrations heads (#6560) * [SIP-14] remove dependency on yarn in favor of npm (#6541) * [SIP-14] remove dependency on yarn in favor of npm Find more details here #6217 * Add core-js to dev deps * lint * fix cypress * break down cypress_build.sh * fix cypress * Use 'npm ci' in place of 'npm install' * Bump npm req to >=6.5.0 * Enforcing npm@'>=6.5.0' * Using latest lts of nvm * Make cache settings global * remove uneeded 'npm run test' as 'npm run cover' takes care of that * Prefix with 'time' * Trying to upgrade babel-eslint instead of downgrading * upgrading babel-polyfill to '@babel/polyfill' * Moving to babel 7 * remove unused packages * Make boto3/botocore installation optional (#6540) * Make boto3 installation optional * pylinting * [SQL Lab] Allow running multiple statements (#6112) * Allow running multiple statements from SQL Lab * fix tests * More tests * merge heads * fix heads * Adding missing slash (#6567) * [refactor] moving some datasource-related code to the frontend (#5769) * [refactor] moving some datasource-related code to the frontend * fix js tests * fix tests * fix test * json_iso_dttm_ser use for TableViz (#6563) * fix #6530 and add some other chinese translation (#6569) * Fix: updated required cryptography version to 2.4.2 to resolve #6509 (#6516) * Fix: updated required cryptography version to 2.4.2 to resolve #6509 * Fix: updated cryptography version to 2.4.2 to resolve #6509 * Fix: updated cryptography version to 2.4.2 to resolve #6509 * Revert "Fix: updated required cryptography version to 2.4.2 to resolve #6509" This reverts commit b57b08c. * Fix: updated cryptography version to 2.4.2 * Fix multi/dual when no right y axis (#6571) * [design] use angle icons instead of carets for expandable panels (#6564) * [design] use angle icons instead of carets for expandable panels Also moving to the right to conform to the material design specs * Fix cypress test * fixing issue #6572 with Oracle date handling (#6580) * fix Oracle engine specs for dates issue #6572 * fix Oracle engine specs for dates issue #6572 * fix Oracle engine specs for dates issue #6572, removing comment * ng a trailing space * Change margin for slice description in charts for the dashboard view (#6575) Changed margin for the slices description in dashboard view, previously the margin was 5px for all the sides but for improve the view just apply margin for top and bottom. * [bugfix] moving from reactable to reactable-arc fork (#6576) * Using batch_op in db migration 0b1f1ab473c0 (#6581) As I needed to downgrade from db migration 0b1f1ab473c0, I realized I needed to use batch_op against SQLLite. * Finish move to babel 7 (#6573) * Finish move to babel 7 * Bump jest to 23.6.0 * Address comments * Enhance Docker (#6504) Refactored deprecated functionalities: - Used 'celery worker' command instead of 'superset worker' which is deprecated since 0.26.0 - Used 'gunicorn' command instead of 'superset runserver' * add chinese translate (#6592) * Fix bug: some word not translate in js. (#6598) delete the 'null,' in messages.json, so that word can be translate in js. * shift labels down along y-axis (#6596) * Fix 'Uncaught TypeError: Cannot read property 'value' of undefined' #6556 (#6574) closes #6556 * added get_data ; * added cache logic * added cache logic * added cache key logic * add missing part * revised the metric part * added to_dict * fix lint * fix lint * fix lint * fix lint * fix lint * fix flake8 * fix invalid name * fix invalid name * fix invalid name * enable worldcloid * working version * rebase * fixed expore link issue * add endpoint * added migration script WIP * fixed lint * remove migration script * fix flake8 * fix tesT * fix tesT * fix tesT * fix typo * fixed metric format * fixed metric format * fixed metric format * fixed tests * fixed test * fixed druid test * fixed more tests * fixed more tests * fix test for viz * fixed js tests * fixed ts test * fixed ts test * fixed cypress test * fixed the metric format in filterViz

[SIP-14] remove dependency on yarn in favor of npm

7fcddc4

Find more details here apache#6217

mistercrunch force-pushed the remove_yarn branch from 9f750f1 to 7fcddc4 Compare December 16, 2018 18:15

mistercrunch added 2 commits December 16, 2018 13:03

Add core-js to dev deps

ed4893c

lint

6704c1f

mistercrunch added 2 commits December 16, 2018 13:59

fix cypress

cf97556

break down cypress_build.sh

7968bd6

mistercrunch force-pushed the remove_yarn branch 2 times, most recently from 1f90025 to 92d4564 Compare December 17, 2018 00:12

fix cypress

eae522c

mistercrunch force-pushed the remove_yarn branch from 92d4564 to eae522c Compare December 17, 2018 00:36

victornoel reviewed Dec 17, 2018

View reviewed changes

kristw added the #code-quality label Dec 17, 2018

Use 'npm ci' in place of 'npm install'

6b48d7d

Bump npm req to >=6.5.0

1027069

mistercrunch commented Dec 18, 2018

View reviewed changes

tox.ini Show resolved Hide resolved

Enforcing npm@'>=6.5.0'

8d304a7

mistercrunch force-pushed the remove_yarn branch from 70ef274 to 5d879be Compare December 18, 2018 05:55

Using latest lts of nvm

c2b18b1

mistercrunch force-pushed the remove_yarn branch from 5d879be to c2b18b1 Compare December 18, 2018 05:58

mistercrunch added 2 commits December 17, 2018 22:11

Make cache settings global

8401a06

remove uneeded 'npm run test' as 'npm run cover' takes care of that

73fb85d

Prefix with 'time'

a321a59

phoenix24 approved these changes Dec 18, 2018

View reviewed changes

kristw reviewed Dec 18, 2018

View reviewed changes

Trying to upgrade babel-eslint instead of downgrading

496a3ce

kristw mentioned this pull request Dec 19, 2018

Address core-js dependency in @superset-ui/core apache-superset/superset-ui#70

Closed

mistercrunch added 3 commits December 18, 2018 18:00

upgrading babel-polyfill to '@babel/polyfill'

a72661a

Moving to babel 7

6438917

remove unused packages

79fc45c

betodealmeida merged commit 60ccf3e into apache:master Dec 21, 2018

mistercrunch deleted the remove_yarn branch December 21, 2018 18:29

leogreyz mentioned this pull request Dec 26, 2018

docker babel-node not found #6489

Closed

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.34.0 labels Feb 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SIP-14] remove dependency on yarn in favor of npm #6541

[SIP-14] remove dependency on yarn in favor of npm #6541

mistercrunch commented Dec 16, 2018 •

edited

Loading

codecov-io commented Dec 16, 2018 •

edited

Loading

victornoel Dec 17, 2018

victornoel Dec 17, 2018

victornoel Dec 17, 2018

victornoel Dec 17, 2018

mistercrunch commented Dec 18, 2018

phoenix24 commented Dec 18, 2018 •

edited

Loading

phoenix24 commented Dec 18, 2018

phoenix24 left a comment

kristw Dec 18, 2018

mistercrunch Dec 19, 2018

mistercrunch Dec 19, 2018

kristw Dec 19, 2018

mistercrunch Dec 19, 2018

mistercrunch commented Dec 19, 2018

mistercrunch commented Dec 21, 2018

betodealmeida commented Dec 21, 2018

[SIP-14] remove dependency on yarn in favor of npm #6541

[SIP-14] remove dependency on yarn in favor of npm #6541

Conversation

mistercrunch commented Dec 16, 2018 • edited Loading

codecov-io commented Dec 16, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mistercrunch commented Dec 18, 2018

phoenix24 commented Dec 18, 2018 • edited Loading

phoenix24 commented Dec 18, 2018

phoenix24 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mistercrunch commented Dec 19, 2018

mistercrunch commented Dec 21, 2018

betodealmeida commented Dec 21, 2018

mistercrunch commented Dec 16, 2018 •

edited

Loading

codecov-io commented Dec 16, 2018 •

edited

Loading

phoenix24 commented Dec 18, 2018 •

edited

Loading