Internal Server Error because build --dev should run commands via pipenv

[fenekku]

Steps to reproduce error:

0- install invenio-cli globally (via sudo pip install invenio-cli or more recommended via pipx) 1- $ invenio-cli init --flavour=RDM 2- $ cd <your project short name> 3- $ invenio-cli build --pre --dev --verbose Expected 4- assets are collected and built Got 4- Console output:

$ invenio-cli build --pre --dev --verbose
Building RDM application...
Locking dependencies...
Bootstrapping server...
Checking that dependencies are locked...
Installing dependencies...
Collecting assets......
Usage: cli [OPTIONS] COMMAND [ARGS]...
Try "cli --help" for help.

Error: No such command "collect".

Building assets......
Usage: cli [OPTIONS] COMMAND [ARGS]...
Try "cli --help" for help.

Error: No such command "webpack".

Creating statics folder...
Statics directory already exsits...
Creating development services...

This is because

@with_appcontext
def build_assets(log_config):
    """Build assets."""
    cli = create_cli()
    runner = current_app.test_cli_runner()
    # Collect
    run_command(cli, runner, "collect -v", message="Collecting assets...",
                verbose=log_config.verbose)

    # Build using webpack
    run_command(cli, runner, "webpack buildall",
                message="Building assets...", verbose=log_config.verbose)

calls run_command directly rather than subprocess.call(command, stdout=logpipe, stderr=logpipe) where command = ['pipenv', 'run', 'collect', '--verbose'] much like _bootstrap_dev .

Maybe this should be merged with #24 (and remove "good first ussue" from that one)

[ppanero]

Previously (first release) the --dev version was using subprocess calls to run pipenv run calls. It was proven to be really slow (in the order of minutes compared to now).

Therefore, it was agreed to run inside an application context, which uses afterwards the command runner (see run_command function). The main drawback of this is the lack of support for having the CLI and the RDM application in different venvs (It was not considered).

Personally, I consider coming back to subprocess calls with pipenv run a step backwards. There is a clean way to solve this, which would be to somehow spawn an interpreter that loaded the current Invenio config and then run commands in there. That way we could profit of having the CLI separated and still load only once the application context (and avoid pipenv overhead for each operation).

Nonetheless, this is a hard task and not doable at the moment. After discussing with the Zenodo dev team, we agreed to postpone this fix until there is time to be allocated to it and fix it the proper way. For now, we would just need to document that for the --dev instance we need to:

• Create + activate a venv
• Install CLI
• init, etc. using the CLI

Having the CLI installed per venv/instance is an overhead, but is much samller than waiting 3-4 minutes to setup --force. Plus --dev taget devs, so the steps in the above list should be familiar.

What do you think @fenekku ?

[fenekku]

Agreed. I've changed the wiki to reflect that in part last week. Take another look to see if adding the virtualenv/pipenv run in the command line code sections would be helpful.

I am curious about the potential solution

spawn an interpreter that loaded the current Invenio config and then run commands in there

What does that do? Wouldn't we then still be loading a virtualenv? (we have to because that's where the commands are right?)

[ppanero]

Agreed. I've changed the wiki to reflect that in part last week. Take another look to see if adding the virtualenv/pipenv run in the command line code sections would be helpful.

I would disagree with recommending pipenv to install CLI as modified in the Wiki. That's why I mentioned here. Basically, it is only needed for the --local case, since --containers works without issue inside a container. Therefore, I would just suggest the creation of a virtualenv in advance, suggesting the installation of a library via pipenv is kind of decieving the purpose of it (It is meant for applications, it will create a Pipfile wherever you are, etc. I would personally revert this changes, but didn't to not conflict :). That is why I expalined the trade off above.

What does that do? Wouldn't we then still be loading a virtualenv? (we have to because that's where the commands are right?)

We can discuss this in the future, but basically we would be "reimplementing" the commands and loading configuration so we do not need them. It was just an idea from a short brainstorming with the Zenodo team. We can analyze it further when we reach this point (January/Februray).

[fenekku]

Basically, it is only needed for the --local case, since --containers works without issue inside a container. Therefore, I would just suggest the creation of a virtualenv in advance, suggesting the installation of a library via pipenv is kind of decieving the purpose of it (It is meant for applications

... let's talk about that tomorrow too if we can :sweat_smile: My main thought being that:

I agree that the local virtualenv (pipenv approach) is only needed for --local ... but if a virtualenv is created, then the --containers works and --local works, as opposed to only 1 working otherwise. That is better, no?