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

BLD: Refresh gitpod build files #56375

Closed
wants to merge 11 commits into from
Closed

Conversation

lithomas1
Copy link
Member

  • closes #xxxx (Replace xxxx with the GitHub issue number)
  • Tests added and passed if fixing a bug or adding a new feature
  • All code checks passed.
  • Added type annotations to new arguments/methods/functions.
  • Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

@lithomas1 lithomas1 added the CI Continuous Integration label Dec 7, 2023
&& rm -rf /var/lib/apt/lists/*

# Install miniconda
ENV CONDA_DIR /opt/conda
Copy link
Member

Choose a reason for hiding this comment

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

Not opposed but whats the advantage of running conda within Docker versus just using docker for the virtualization?

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess to install new packages?

For some reason I can't do stuff like apt install in gitpod.
(sudo doesnt work for me)

In general, I think it's preferred to have stuff installed from conda over pip, too.

Copy link
Member

Choose a reason for hiding this comment

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

Does apt-get install work?

Copy link
Member

Choose a reason for hiding this comment

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

Also on the note of conda versus pip - conda is a pretty big install. Most images try to be small and easy to deploy; I would imagine this makes the image significantly larger than the base

Copy link
Member

Choose a reason for hiding this comment

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

Are users expected to modify this environment? If so I think installing conda would be nice for ease of use. But since this is for development anyways and not a service having conda here would be OK to me

Copy link
Member Author

Choose a reason for hiding this comment

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

Does apt-get install work?

I don't think so?
It's been a while since I last used gitpod. Sorry for the late reply.

Copy link
Member Author

Choose a reason for hiding this comment

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

Are users expected to modify this environment? If so I think installing conda would be nice for ease of use. But since this is for development anyways and not a service having conda here would be OK to me

I tend to want to install new packages at least
(e.g. things like a profiler, or trying out different versions of dependencies, like numpydev, etc.)

ENV PATH=$CONDA_DIR/bin:$PATH

# init conda
RUN conda init
Copy link
Member

Choose a reason for hiding this comment

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

Could you combine these RUN statements to reduce the image layers?

@mroeschke mroeschke mentioned this pull request Jan 8, 2024
2 tasks
# remote clone
ARG BASE_CONTAINER="pandas/pandas-dev:latest"
FROM gitpod/workspace-base:latest as clone
FROM condaforge/miniforge3:23.3.1-1
Copy link
Contributor

Choose a reason for hiding this comment

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

One potential issue with using the conda image here is that it is pinned to a specific version. This can lead to Gitpod breaking like in #53685 when the version of conda becomes incompatible with the version of python and other dependencies the environment.yml file.

I think building from the base repo's dockerfile without conda is probably the simpler and easier to maintain approach, as the gitpod instance will update with the base repo's dockerfile. If we were to go down that route, the gitpod folder could actually be deleted entirely, and a .gitpod.yml file is all that is required.

Copy link
Member Author

Choose a reason for hiding this comment

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

The reason I switched to conda from Docker is that docker doesn't provide a way to install packages (I don't think there is any sudo in the gitpod container - at least apt-get/pip don't work for me).

I think Docker might also have the same problem since the packages installed will be cached and won't update unless you recreate the gitpod instance.

I avoid the problem here by updating the conda packages on install based on the environment.yml. I'm happy to switch back to Docker if there's a way to do the same there.

Copy link
Member

Choose a reason for hiding this comment

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

I think most Docker images do not include sudo. What error were you seeing when doing something like apt-get update && apt-get install -y <some_package>?

Copy link
Member Author

Choose a reason for hiding this comment

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

Something like this

gitpod@lithomas1-pandas-1tcur93d5vv:/workspace/pandas$ apt install build-essential
E: Could not open lock file /var/lib/dpkg/lock-frontend - open (13: Permission denied)
E: Unable to acquire the dpkg frontend lock (/var/lib/dpkg/lock-frontend), are you root?
gitpod@lithomas1-pandas-1tcur93d5vv:/workspace/pandas$ sudo apt install build-essential
bash: sudo: command not found

Copy link
Member

Choose a reason for hiding this comment

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

Hmm strange...and you are running that between the

USER root

....

USER gitpod

commands in the image right? I think anything in between those should be running under the root user account

Copy link
Member Author

@lithomas1 lithomas1 Jan 10, 2024

Choose a reason for hiding this comment

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

No, this is when the workspace is started up, not in the Dockerfile.

Copy link
Member

Choose a reason for hiding this comment

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

It looks like the Dockerfile intentionally runs under a non-priviledged user. So you'd have to install system libraries before that or remove the USER gitpod statement in gitpod.Dockerfile (no idea if that is a good idea for gitpod or not)

Copy link
Contributor

This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this.

@github-actions github-actions bot added the Stale label Feb 10, 2024
@mroeschke
Copy link
Member

Going to close to clear the queue, but feel free to reopen if you want to circle back

@mroeschke mroeschke closed this Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants