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

chore(docker): move mysql os-level deps (GPL) to dev image only #29771

Merged
merged 7 commits into from
Sep 15, 2024

Conversation

mistercrunch
Copy link
Member

Since mysql client libs are GPL-licensed, we don't want to package them in our default images. Prior to this PR, the default-libmysqlclient-dev apt-get package would get installed in the lean docker image, though the pypi mysqlclient package that depends on this os-level dep was only installed in the dev layer.

After this PR, all GPL/mysql related package won't be in lean, but still be in dev for convenience and CI script that test against mysql.

@mistercrunch mistercrunch added the install:docker Installation - docker container label Jul 29, 2024
@michael-s-molina michael-s-molina linked an issue Jul 30, 2024 that may be closed by this pull request
3 tasks
@mistercrunch mistercrunch force-pushed the no_mysql_lean branch 2 times, most recently from 66c39c8 to e9f95c8 Compare July 30, 2024 19:02
@github-actions github-actions bot added the github_actions Pull requests that update GitHub Actions code label Jul 30, 2024
@eschutho eschutho added the v4.1 Label added by the release manager to track PRs to be included in the 4.1 branch label Jul 30, 2024
@rusackas
Copy link
Member

Curious to see if --no-cache-dir will affect our PyPI stats and smooth things out at all.

Also, do you have any sense of what's causing the memory increase to the point of needing swap space? Probably unrelated to this, but a little disconcerting.

Copy link
Member

@rusackas rusackas left a comment

Choose a reason for hiding this comment

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

docker-build step is failing, which seems important for this PR 😅

@mistercrunch
Copy link
Member Author

Right, not sure what's up with it, it looks like the job is getting killed, presumably because it's abusing memory usage (?) The confusing thing is we're actually installing less things in the step that fails.

I tried a few things already:

  • setting up swap space, doesn't seem to help, result of following a GPT-generated idea, I'll probably remove that part of the PR
  • was looking for ways to reduce the parallelism in the docker build, currently it's building the superset-node (super memory heavy) while it builds the lean layer (pip install requirements/base.txt), but didn't find a clear/easy way to do it. Was looking for either a docker CLI flag or some env var, but nothing seemed to work, GPT hallucinated a few solutions but that didn't work

I'm wondering what in this PR is pushing the memory usage (assuming that's why the job gets killed) over the edge, and it seems to indicate that the master is probably fragile and on the edge too, meaning that this CI step is likely to fail on master soon too next time we touch something related.

Next steps/ideas:

  • maybe running a docker command to build the superset-node target first, and then push for the other command which would reuse the cached layers from the first command
  • find some other way to limit parallelism
  • find a way to get more memory in GHA?

@sadpandajoe sadpandajoe removed the v4.1 Label added by the release manager to track PRs to be included in the 4.1 branch label Aug 23, 2024
Since mysql client libs are GPL-licensed, we don't want to package them in our default images. Prior to this PR, the `default-libmysqlclient-dev` apt-get package would get installed in the `lean` docker image, though the pypi `mysqlclient` package that depends on this os-level dep was only installed in the `dev` layer.

After this PR, all GPL/mysql related package won't be in `lean`, but still be in `dev` for convenience and CI script that test against mysql.
@github-actions github-actions bot removed the github_actions Pull requests that update GitHub Actions code label Sep 13, 2024
@rusackas rusackas merged commit bcbd679 into master Sep 15, 2024
33 checks passed
@rusackas rusackas deleted the no_mysql_lean branch September 15, 2024 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
install:docker Installation - docker container size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docker image does not have mysqlclient
4 participants