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

commands: assets update/watch/install-module/watch-module & shell & pyshell #173

Merged
merged 4 commits into from Nov 5, 2020

Conversation

lnielsen
Copy link
Member

@lnielsen lnielsen commented Nov 4, 2020

  • Moves invenio-cli update to invenio-cli assets update

  • Adds options invenio-cli assets update --force --development:

    • --force: Will clean the assets folder including node modules and install/build the webpack project.
    • --development: Will symlink files (by setting FLASK_ENV=development)
  • Adds command invenio-cli assets watch to watch the instance's webpack project for changes (running pipenv run invenio webpack run start).

  • Adds command invenio-cli assets install-module which will install, build, and link a JavaScript module (using pynpm module).

  • Adds command invenio-cli assets watch-module which watch changes on a JavaScript module.

  • Adds command invenio-cli shell which will start a virtualenv shell.

  • Adds command invenio-cli pyshell which will start a Python application shell.

  • Adds option invenio-cli install --pre --development to set FLASK_ENV=developement and thereby symlink assets

Addresses #170


WIth this you can now:

invenio-cli init rdm -c master
invenio-cli install --pre
# Next line to be replaced with "invenio-cli ext module-install ~/src/invenio-rdm-records"
pipenv run pip install -e ~/src/invenio-rdm-records -e ~/src/invenio-app-rdm 
invenio-cli assets update --force --development
invenio-cli services
invenio-cli demo

# New shell
invenio-cli run

# New shell
invenio-cli assets watch-module ~/src/react-invenio-deposit --link

# New shell
invenio-cli assets watch

Now, live happily ever after with live reloading when editing files.


Note 1

I didn't go with invenio-cli ext js-install as a Python module install and React module install works very differently, so I personally felt it was more appropriate to have an assets command which operates purely on the assets.

Note 2

We could likely integrate invenio-cli assets update --force --development into invenio-cli ext module-install ~/src/invenio-rdm-records. For instance:

invenio-cli ext module-install ~/src/invenio-rdm-records ~/src/invenio-app-records

Would run:

pipenv run pip install -e ~/src/invenio-rdm-records -e ~/src/invenio-app-rdm 
invenio-cli assets update --force --development

You could disable the rebuild of assets with :

invenio-cli ext module-install --without-assets ~/src/invenio-rdm-records

@lnielsen lnielsen added this to In progress in InvenioRDM October Board Nov 4, 2020
* Changes the update command to allow specifying the FLASK_ENV as
  well as cleaning the assets folder prior to building and installing.

* Changes the parameters for the updated command.

* Adds a new command to start assets watching.
@lnielsen lnielsen marked this pull request as ready for review November 4, 2020 20:48
@ppanero
Copy link
Member

ppanero commented Nov 4, 2020

To be crosschecked but if the FLASK_ENV=development is not on the install step it will actually do a hard copy of the assets then the second time (when update) it won't do anything (probably the clean_create_cmd is fixing this, but it requires always --force)

The first time is called here:

self.update_statics_and_assets(force=True)
and the default for the flask_env is production. Since it it a function inside LocalCommands, would it make sense to change default to developement? I would assume that whoever goes through installing locally would develop...

Just a heads up in case someone tries it, I think it is:

invenio-cli assets update --force --development
invenio-cli assets watch

Not invenio.

@coveralls
Copy link

coveralls commented Nov 4, 2020

Coverage Status

Coverage increased (+0.2%) to 86.304% when pulling 6068cba on lnielsen:assets into 44035e3 on inveniosoftware:master.

@lnielsen lnielsen changed the title commands: assets update and assets watch commands: assets update/watch/install-module/watch-module & shell & pyshell Nov 4, 2020
@lnielsen
Copy link
Member Author

lnielsen commented Nov 4, 2020

To be crosschecked but if the FLASK_ENV=development is not on the install step it will actually do a hard copy of the assets [...]

So, I've added (setting FLASK_ENV=development during install):

invenio-cli install --development

However, as soon as you install an existing module in dev mode (e.g. pipenv run pip install -e ~/src/invenio-app-rdm), you need to force rebuild all assets (because the invenio webpack create is not smart enough to wipe an existing wrong link).

I think it makes sense to stay with production as default, since most users will just need to edit the assets in their own instance (which are always symlinked).

Just a heads up in case someone tries it, I think it is:

Correct, I've edited the description accordingly.

@lnielsen lnielsen moved this from In progress to In Review in InvenioRDM October Board Nov 4, 2020
* Adds command 'invenio-cli assets install-module' which will install,
  build, and link a JavaScript module.

* Adds command 'invenio-cli assets watch-module' which watch changes on
  a JavaScript module.

* Adds command 'invenio-cli shell' which will start a virtualenv shell.

* Adds command 'invenio-cli pyshell' which will Python application
  shell.
@lnielsen lnielsen merged commit c8ca360 into inveniosoftware:master Nov 5, 2020
InvenioRDM October Board automation moved this from In Review to Done Nov 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants