Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 17 commits into from
Dec 21, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 11 additions & 31 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,42 +5,24 @@ jobs:
- language: python
python: 3.6
env: TOXENV=cypress-dashboard
cache:
pip: true
yarn: true
directories:
- ~/.npm
- ~/.cache
services:
- redis-server
before_install:
- nvm install 8.9
- nvm install 10.14.2
- language: python
python: 3.6
env: TOXENV=cypress-explore
cache:
pip: true
yarn: true
directories:
- ~/.npm
- ~/.cache
services:
- redis-server
before_install:
- nvm install 8.9
- nvm install 10.14.2
- language: python
python: 3.6
env: TOXENV=cypress-sqllab
cache:
pip: true
yarn: true
directories:
- ~/.npm
- ~/.cache
services:
- redis-server
before_install:
- nvm install 8.9
- nvm install 10.14.2
- language: python
python: 3.6
env: TOXENV=py36-mysql
Expand All @@ -51,17 +33,10 @@ jobs:
- mysql -u root -e "DROP DATABASE IF EXISTS superset; CREATE DATABASE superset DEFAULT CHARACTER SET utf8 COLLATE utf8_unicode_ci"
- mysql -u root -e "CREATE USER 'mysqluser'@'localhost' IDENTIFIED BY 'mysqluserpassword';"
- mysql -u root -e "GRANT ALL ON superset.* TO 'mysqluser'@'localhost';"
- language: node_js
node_js: 8.9
cache:
yarn: true
- language: python
env: TOXENV=javascript
before_install:
- cd superset/assets
install:
- yarn install --frozen-lockfile
script:
- npm run lint
- npm run cover
- nvm install 10.14.2
- language: python
python: 3.6
env: TOXENV=flake8
Expand Down Expand Up @@ -89,3 +64,8 @@ script:
- tox
after_success:
- codecov
cache:
pip: true
directories:
- ~/.npm
- ~/.cache
17 changes: 6 additions & 11 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -253,12 +253,6 @@ First, be sure you are using recent versions of NodeJS and npm. Using [nvm](http

#### Prerequisite

If needed, install yarn

```bash
npm install -g yarn
```

#### Installing Dependencies

Install third-party dependencies listed in `package.json`:
Expand All @@ -267,8 +261,8 @@ Install third-party dependencies listed in `package.json`:
# From the root of the repository
cd superset/assets

# Install dependencies
yarn install
# Install dependencies from `package-lock.json`
npm ci
```

#### Building
Expand Down Expand Up @@ -301,7 +295,9 @@ npm run sync-backend

#### Updating NPM packages

After adding or upgrading an NPM package by changing `package.json`, you must run `yarn install`, which will regenerate the `yarn.lock` file. Then, be sure to commit the new `yarn.lock` so that other users' builds are reproducible. See [the Yarn docs](https://yarnpkg.com/blog/2016/11/24/lockfiles-for-all/) for more information.
Use npm in the prescribed way, making sure that
`superset/assets/package-lock.json` is updated according to `npm`-prescribed
best practices.

#### Feature flags

Expand All @@ -328,7 +324,7 @@ tox -e flake8

# for javascript
cd superset/assets
yarn install
npm ci
npm run lint
```

Expand Down Expand Up @@ -372,7 +368,6 @@ We use [Jest](https://jestjs.io/) and [Enzyme](http://airbnb.io/enzyme/) to test

```bash
cd superset/assets
yarn install
npm run test
```

Expand Down
6 changes: 6 additions & 0 deletions UPDATING.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@
This file documents any backwards-incompatible changes in Superset and
assists people when migrating to a new version.

## Superset 0.31.0
From 0.31.0 onwards, we recommend not using the npm package `yarn` in
favor of good old `npm install`. While yarn should still work just fine,
you should probably align to guarantee builds similar to the ones we
use in testing and across the community in general.

## Superset 0.29.0
* India was removed from the "Country Map" visualization as the geojson
file included in the package was very large
Expand Down
15 changes: 4 additions & 11 deletions contrib/docker/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,6 @@ RUN apt-get install -y vim less postgresql-client redis-tools
RUN curl -sL https://deb.nodesource.com/setup_10.x | bash - \
&& apt-get install -y nodejs

# https://yarnpkg.com/lang/en/docs/install/#debian-stable
RUN curl -sS https://dl.yarnpkg.com/debian/pubkey.gpg | apt-key add - \
&& echo "deb https://dl.yarnpkg.com/debian/ stable main" | tee /etc/apt/sources.list.d/yarn.list \
&& apt-get update \
&& apt-get install -y yarn

WORKDIR /home/superset

COPY requirements.txt .
Expand All @@ -48,11 +42,10 @@ ENV PATH=/home/superset/superset/bin:$PATH \
PYTHONPATH=/home/superset/superset/:$PYTHONPATH

RUN cd superset/assets \
&& yarn --non-interactive --frozen-lockfile --link-duplicates \
&& yarn run sync-backend \
&& yarn run build \
&& rm -rf node_modules \
&& yarn cache clean
&& npm ci
&& npm run sync-backend \
&& npm run build \
&& rm -rf node_modules

COPY contrib/docker/docker-init.sh .
COPY contrib/docker/docker-entrypoint.sh /entrypoint.sh
Expand Down
4 changes: 2 additions & 2 deletions contrib/docker/docker-entrypoint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 ci && npm run sync-backend)
(cd superset/assets/ && npm run dev) &
flask run -p 8088 --with-threads --reload --debugger --host=0.0.0.0
elif [ "$SUPERSET_ENV" = "production" ]; then
superset worker &
Expand Down
11 changes: 9 additions & 2 deletions superset/assets/cypress_build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,17 @@ set -e
cd "$(dirname "$0")"

#run all the python steps in a background process
(time /home/travis/build/apache/incubator-superset/superset/bin/superset db upgrade; time /home/travis/build/apache/incubator-superset/superset/bin/superset load_test_users; /home/travis/build/apache/incubator-superset/superset/bin/superset load_examples --load-test-data; time /home/travis/build/apache/incubator-superset/superset/bin/superset init; echo "[completed python build steps]"; flask run -p 8081 --with-threads --reload --debugger) &
time superset db upgrade
time superset load_test_users
time superset load_examples --load-test-data
time superset init
echo "[completed python build steps]"
flask run -p 8081 --with-threads --reload --debugger &

#block on the longer running javascript process
(time yarn install --frozen-lockfile; time npm run build; echo "[completed js build steps]")
time npm ci
time npm run build
echo "[completed js build steps]"

CYPRESS_PATH='cypress/integration/'${1}'/*'
time npm run cypress run -- --spec "$CYPRESS_PATH" --record false --config video=false
Expand Down
10 changes: 4 additions & 6 deletions superset/assets/js_build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@ set -e
cd "$(dirname "$0")"
npm --version
node --version
npm install -g yarn
yarn
npm run lint
npm run test
npm run build
npm run cover
time npm ci
time npm run lint
time npm run cover # this also runs the tests, so no need to 'npm run test'
time npm run build
Loading