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

Pipenv #657

Closed
wants to merge 21 commits into from
Closed

Pipenv #657

wants to merge 21 commits into from

Conversation

@nickumia-reisys
Copy link
Contributor Author

Unanswered questions:

  • Do we want to consolidate pipenv for cloud.gov use too?
  • Why does this seem not much better than before?

Copy link
Contributor

@jbrown-xentity jbrown-xentity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we'd need to do something different here...

Comment on lines 1 to 2
alembic==1.0.0
async-timeout==4.0.2
Babel==2.9.1
Beaker==1.11.0
argparse==1.4.0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally this whole file is removed...

Also, add metrics dashboard back in
ckan/Dockerfile Outdated
@@ -31,11 +24,18 @@ ENV PIP_SRC=${SRC_DIR}
ENV CKAN_STORAGE_PATH=/var/lib/ckan
RUN mkdir -p $CKAN_STORAGE_PATH $APP_DIR

ADD Pipfile Pipfile.lock ${APP_DIR}/
ENV PIPENV_PIPFILE=/${APP_DIR}/Pipfile
RUN pip3 install --upgrade pipenv && pipenv install --ignore-pipfile
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we ignoring pipfile here? I think we want to use that...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're ignoring pipfile in favor of pipfile.lock which is the hard set of requirements.

Comment on lines 3 to 6
set -o errexit
set -o pipefail

venv=$(mktemp -d)
pip3 install pipenv
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we want this file anymore, but we should use the standard pipenv pinning and upgrading process

Comment on lines 29 to 31
done

echo "Enabling debug mode"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See lines 19-26 in this file, I think we'll have to do something a little different...

@@ -10,7 +10,7 @@ DIR="$(dirname "${BASH_SOURCE[0]}")"
# Threads RAM requirement unknown at this time
# exec newrelic-admin run-program gunicorn -c "$DIR/gunicorn.conf.py" --worker-class gevent --paste $CKAN_INI "$@"
if [[ "$CKAN_SITE_URL" = "http://ckan:5000" ]]; then
exec newrelic-admin run-program gunicorn "wsgi:application" --config "$DIR/gunicorn.conf.py" -b "0.0.0.0:$PORT" --chdir $DIR --timeout 120 --workers 2
exec pipenv run newrelic-admin run-program gunicorn "wsgi:application" --config "$DIR/gunicorn.conf.py" -b "0.0.0.0:$PORT" --chdir $DIR --timeout 120 --workers 2
else
exec newrelic-admin run-program gunicorn "wsgi:application" --config "$DIR/gunicorn.conf.py" -b "0.0.0.0:$PORT" --chdir $DIR --timeout 120 --worker-class gevent --workers 4 --threads 4 --forwarded-allow-ips='*'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Equivalent pipenv run needed here possibly; or maybe not? would require cloud.gov testing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I'll look into this.

I still have to test if pipenv can be manipulated manually instead of from inside of docker
This switches gears a bit and keepsthe optimal pip performance for local development and build and testing.  And uses the management optimal pipenv for cloud.gov 'vendoring' which just ensures that compatible versions are referenced in the cloud.gov environment
@nickumia-reisys
Copy link
Contributor Author

If this is ever re-visited for future work, there is still a lot of work to do with ensuring all of the dependencies are cloud.gov-compatible. There was no concrete path forward from the current state.

@btylerburton btylerburton deleted the pipenv branch April 14, 2023 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants