Workflow fixes

[robertbartel]

Several miscellaneous fixes found while testing the DMOD model exec workflow and other related supplementary CLI commands.

[aaraney]

Had one question that should be super easy to address. Lets get this on the road though. Thanks for making these changes @robertbartel!

[aaraney]

Is it intentional not to activate the venv after creating it here? So, should this instead be:

Suggested change

	RUN python3 -m venv $VIRTUAL_ENV && pip3 install numpy
	RUN python3 -m venv $VIRTUAL_ENV && source $VIRTUAL_ENV/bin/activate && pip3 install numpy

[robertbartel]

Pretty sure it is effectively active if VIRTUAL_ENV is set in the environment (eh, maybe PATH needs updating too, but that's also done).

[aaraney]

I think that is just used for bookkeeping so that the added deactivate shell function can remove $VIRTUAL_ENV from $PATH.

[aaraney]

Here is what an venv/bin/activate script looks like (on mac):

activate script

# This file must be used with "source bin/activate" *from bash*
# you cannot run it directly

deactivate () {
    # reset old environment variables
    if [ -n "${_OLD_VIRTUAL_PATH:-}" ] ; then
        PATH="${_OLD_VIRTUAL_PATH:-}"
        export PATH
        unset _OLD_VIRTUAL_PATH
    fi
    if [ -n "${_OLD_VIRTUAL_PYTHONHOME:-}" ] ; then
        PYTHONHOME="${_OLD_VIRTUAL_PYTHONHOME:-}"
        export PYTHONHOME
        unset _OLD_VIRTUAL_PYTHONHOME
    fi

    # This should detect bash and zsh, which have a hash command that must
    # be called to get it to forget past commands.  Without forgetting
    # past commands the $PATH changes we made may not be respected
    if [ -n "${BASH:-}" -o -n "${ZSH_VERSION:-}" ] ; then
        hash -r 2> /dev/null
    fi

    if [ -n "${_OLD_VIRTUAL_PS1:-}" ] ; then
        PS1="${_OLD_VIRTUAL_PS1:-}"
        export PS1
        unset _OLD_VIRTUAL_PS1
    fi

    unset VIRTUAL_ENV
    if [ ! "${1:-}" = "nondestructive" ] ; then
    # Self destruct!
        unset -f deactivate
    fi
}

# unset irrelevant variables
deactivate nondestructive

VIRTUAL_ENV="/home/user/docker-py/venv"
export VIRTUAL_ENV

_OLD_VIRTUAL_PATH="$PATH"
PATH="$VIRTUAL_ENV/bin:$PATH"
export PATH

# unset PYTHONHOME if set
# this will fail if PYTHONHOME is set to the empty string (which is bad anyway)
# could use `if (set -u; : $PYTHONHOME) ;` in bash
if [ -n "${PYTHONHOME:-}" ] ; then
    _OLD_VIRTUAL_PYTHONHOME="${PYTHONHOME:-}"
    unset PYTHONHOME
fi

if [ -z "${VIRTUAL_ENV_DISABLE_PROMPT:-}" ] ; then
    _OLD_VIRTUAL_PS1="${PS1:-}"
    PS1="(venv) ${PS1:-}"
    export PS1
fi

# This should detect bash and zsh, which have a hash command that must
# be called to get it to forget past commands.  Without forgetting
# past commands the $PATH changes we made may not be respected
if [ -n "${BASH:-}" -o -n "${ZSH_VERSION:-}" ] ; then
    hash -r 2> /dev/null
fi

[aaraney]

Hmmm, I assume you are adding this to PATH so venv is effectively always activated. Is there a reason why we can't use the default python environment or instead add venv activation in the entrypoint?

[robertbartel]

Basically, something weird seems to be going on, probably after NOAA-OWP/ngen#755, that's breaking the image build. See CIROH-UA/NGIAB-HPCInfra#12 and CIROH-UA/NGIAB-CloudInfra#137 for more details on this specifically, as others are running into it also.

[aaraney]

Thanks for linking those related issues. Seems like something to do with using a non-virtual environment. My main concern is making it really clear that we are using a virtual environment in the image without needing to come and read the Dockerfile.

[robertbartel]

Fair point. I've opened #630. I don't really want to move away from the default environment, but it seems necessary. But I'd like to eventually take this out, once the underlying ngen build issues are resolved.

[aaraney]

Yeah, this is super weird. Lets just move ahead like you are suggesting and revisit this when we have a fix.

[aaraney]

similar question as above here.