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

Update python tooling image #93

Merged
merged 5 commits into from
Sep 12, 2019
Merged

Update python tooling image #93

merged 5 commits into from
Sep 12, 2019

Conversation

tolusha
Copy link
Contributor

@tolusha tolusha commented Sep 5, 2019

Signed-off-by: Anatoliy Bazko abazko@redhat.com

What does this PR do?

Updates python tooling image:
quay.io/eclipse/che-python-3.6:nightly -> centos/python-36-centos7:1

Signed-off-by: Anatoliy Bazko <abazko@redhat.com>
Signed-off-by: Anatoliy Bazko <abazko@redhat.com>
@@ -16,7 +16,7 @@ components:
-
type: dockerimage
alias: python
image: quay.io/eclipse/che-python-3.6:nightly
image: centos/python-36-centos7:1
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we use python:3.7.4-slim to be consistent with the LS sidecar?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Command to install dependencies for the Django project failed due to permissions

ERROR: Could not install packages due to an EnvironmentError: [Errno 13] Permission denied: '/usr/local/lib/python3.7/site-packages/django'
Consider using the `--user` option or check the permissions.

Copy link
Contributor Author

@tolusha tolusha Sep 6, 2019

Choose a reason for hiding this comment

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

I can use image with python 3.6 for a sidecar container instead.

Copy link
Contributor

@amisevsk amisevsk left a comment

Choose a reason for hiding this comment

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

I don't understand the purpose of this PR. Why are we moving away from using the arbitrary-user-id patched image?

For context, quay.io/eclipse/che-python-3.6:nightly is centos/python-36-centos7:1 with patches to support running on OpenShift.

@tolusha
Copy link
Contributor Author

tolusha commented Sep 9, 2019

@amisevsk
That's a good question
@l0rd ?

Signed-off-by: Anatoliy Bazko <abazko@redhat.com>
@tolusha
Copy link
Contributor Author

tolusha commented Sep 10, 2019

@amisevsk
I've reverted the changes.
This PR is just about fixing description.

@l0rd
Copy link
Contributor

l0rd commented Sep 10, 2019

@tolusha I think the misunderstanding is that, to update the image, you should modify the base_images file https://github.com/eclipse/che-devfile-registry/blob/master/arbitrary-users-patch/base_images#L1, not the meta.yaml directly. That may solve your permission issues as well.

@amisevsk the goal here was to make the python base image consistent with python LS sidecar image eclipse-che/che-theia#427

Copy link
Contributor

@amisevsk amisevsk left a comment

Choose a reason for hiding this comment

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

So that I'm clear, updating base_images to point to python:3.7.4-slim does not work because the pip install command fails? Have we tried updating the devfile to use --user?

Signed-off-by: Anatoliy Bazko <abazko@redhat.com>
@tolusha
Copy link
Contributor Author

tolusha commented Sep 11, 2019

@amisevsk
I think this PR implies creating a new che-python-3.7 repo, could you handle this pls?

Copy link
Contributor

@amisevsk amisevsk left a comment

Choose a reason for hiding this comment

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

Not approving quite yet since I'd like to test the new image before merging.

arbitrary-users-patch/base_images Show resolved Hide resolved
@amisevsk
Copy link
Contributor

I've created the quay.io repo https://quay.io/repository/eclipse/che-python-3.7

Copy link
Contributor

@amisevsk amisevsk left a comment

Choose a reason for hiding this comment

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

Tested python-django devfile with changes and using patched image based off python:3.7.4-slim:

  • I was able to connect a debugger to the running application in the main container
    • I believe the install dependencies task will have to be run every start, since pip --user install installs to ~/.local/, which is not persisted.
  • I get error markers on imports ([pylint] Unable to import 'rest_framework') for non-project imports.

Before merging, please update python devfiles to use quay.io/eclipse/che-python-3.7:nightly

Signed-off-by: Anatoliy Bazko <abazko@redhat.com>
@tolusha tolusha merged commit 1c5a660 into master Sep 12, 2019
@tolusha tolusha deleted the ab/python branch September 12, 2019 09:12
@tsmaeder tsmaeder mentioned this pull request Sep 12, 2019
41 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants