backend: Pin python dependencies #3161

discordianfish · 2020-02-24T11:00:43Z

Builds/tests failed due to broken transident dependencies. This pins the
python dependencies to prevent this in the future.

The requirements.txt was created by running python3 -m pip install tfx==0.21.0 && python3 -m pip freeze in the current python:3.5 image

This change is

discordianfish · 2020-02-24T11:17:50Z

/retest

Bobgy · 2020-02-24T12:45:56Z

Thanks! looks like a first step of #3078.

Bobgy · 2020-02-24T12:49:32Z

backend/requirements.txt

@@ -0,0 +1,124 @@
+absl-py==0.8.1


I think it would be better if

The requirements.txt was created by running python3 -m pip install tfx==0.21.0 && python3 -m pip freeze in the current python:3.5 image

is clearly documented inside code instead of in the PR.

Did you ever try pip-tools? Would a separate requirements.in file for direct deps and a compiled requirements.txt suit this model better?

I just ran into the problem with unpinned dependencies, so fixed it in the most obvious way. I did't add the comment because it didn't seem to matter to me outside the context of the commit, because well, given subsequent runs of that command will yield a different requirements.txt.

Re/ pip-tools, not familiar with it. I don't do much python. Feel free to replace this PR with something better.

I thought that comment is useful because we will probably install more dependencies/uninstall some dependencies from time to time. It will be a disaster to not know which ones are the actual direct dependencies.

/assign @Ark-kun
/assign @numerology
I'm not familiar with python env too. They will be able to help.

discordianfish · 2020-02-25T16:21:54Z

Since I again ran into a failed test due to changed, unpinned dependencies, I'm looking into pip-tools for both. Stay tuned.

discordianfish · 2020-02-25T17:05:58Z

/retest

discordianfish · 2020-02-25T17:49:00Z

Looks like the dependencies as resolved on my local machine somehow don't work in CI.. So I guess we need to run pip-compile in the same docker image.
Any preferences how to update them? Is a comment in the README like this sufficient?

Bobgy

/lgtm
Thanks! This looks great to me.
Only one comment left.

@numerology @Ark-kun for more review.

Bobgy · 2020-02-26T04:38:59Z

backend/Dockerfile

-RUN wget https://bootstrap.pypa.io/get-pip.py && python3 get-pip.py
-RUN python3 -m pip install tfx==0.21.0
+    apt-get install --no-install-recommends -y -q default-jdk python3-setuptools python3-dev && \
+    pip install pip-tools==4.5.0


pip-tools is redundant here, right?

Bobgy · 2020-02-26T05:21:27Z

I think you can document this command, run it in the path with requirements.in:

docker run -v $(pwd):/host python:3.5 bash -c "pip install pip-tools==4.5.0 && pip-compile /host/requirements.in --dry-run"

and copy the generated content to requirements.txt (because if we generates requirements.txt in docker, it will be owned by root).

discordianfish · 2020-02-26T08:48:31Z

Yeah haven't push my latest revision. Should be there in a bit.

discordianfish · 2020-02-26T09:24:36Z

Ok this should be it. I've moved the command to a script and added a README to sample-test as well.

Bobgy

/lgtm
Thank you for your contribution! This is great.

One last comment

Bobgy · 2020-02-26T09:44:08Z

backend/update_requirements.sh

+#!/bin/bash
+set -euo pipefail
+
+exec docker run -v "$PWD:/src" -w /src --rm python:3.5 bash \


Thank you!

Can you duplicate this script in sample-test folder too?
Very useful!

Hrm it can be invoked from sample-test, which is mentioned in the README I've created there. Maybe we should move it to the root of the repo? Or in /bin or /hack or something?

Bobgy · 2020-02-26T10:09:28Z

test/sample-test/Dockerfile

-RUN pip3 install tfx==0.21.0
-
+COPY ./test/sample-test/requirements.txt /python/src/github.com/kubeflow/pipelines/test/sample-test/requirements.txt
+RUN pip3 install -r /python/src/github.com/kubeflow/pipelines/test/sample-test/requirements.txt


@discordianfish actually, this uses a google/cloud-sdk:279.0.0 image, can you try pinning dependency in this image.

Builds/tests failed due to broken transident dependencies. This pins the python dependencies to prevent this in the future.

discordianfish · 2020-03-05T08:48:31Z

Ok tests pass now! @Ark-kun @Bobgy @numerology can you approve (again)?

test/sample-test/requirements.in

Bobgy · 2020-03-05T09:11:43Z

Awesome, great thanks!
/lgtm
/approve

@jingzhang36 for backend approval
~~@Ark-kun @numerology for sample test approval~~

Didn't realize I have permission too for test folder

discordianfish · 2020-03-05T09:40:18Z

/assign @rmgogogo

Let's get this merged before it outdates again :)

jingzhang36 · 2020-03-05T09:53:30Z

/approve

k8s-ci-robot · 2020-03-05T09:53:47Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Bobgy, jingzhang36

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~backend/OWNERS~~ [jingzhang36]
~~test/sample-test/OWNERS~~ [Bobgy]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

* backend: Pin python dependencies Builds/tests failed due to broken transident dependencies. This pins the python dependencies to prevent this in the future. * Add README to test/sample-test

k8s-ci-robot added the size/L label Feb 24, 2020

k8s-ci-robot requested review from IronPan and jingzhang36 February 24, 2020 11:00

discordianfish force-pushed the python-pin-dependencies branch from 084a8e9 to 7ed33a9 Compare February 24, 2020 11:17

Bobgy reviewed Feb 24, 2020

View reviewed changes

k8s-ci-robot assigned Ark-kun and numerology Feb 24, 2020

discordianfish force-pushed the python-pin-dependencies branch from 18b42eb to 661c770 Compare February 25, 2020 17:07

discordianfish mentioned this pull request Feb 25, 2020

minio: Set secure=true to enable TLS by default #3168

Merged

Bobgy reviewed Feb 26, 2020

View reviewed changes

k8s-ci-robot assigned Bobgy Feb 26, 2020

k8s-ci-robot added the lgtm label Feb 26, 2020

discordianfish force-pushed the python-pin-dependencies branch from 661c770 to 894cba4 Compare February 26, 2020 09:23

k8s-ci-robot removed the lgtm label Feb 26, 2020

Bobgy reviewed Feb 26, 2020

View reviewed changes

k8s-ci-robot added the lgtm label Feb 26, 2020

Bobgy reviewed Feb 26, 2020

View reviewed changes

discordianfish force-pushed the python-pin-dependencies branch from 894cba4 to 08aea77 Compare February 26, 2020 10:32

k8s-ci-robot removed the lgtm label Feb 26, 2020

discordianfish force-pushed the python-pin-dependencies branch 3 times, most recently from 0ab6975 to f879307 Compare March 4, 2020 16:07

backend: Pin python dependencies

56c2c0d

Builds/tests failed due to broken transident dependencies. This pins the python dependencies to prevent this in the future.

discordianfish force-pushed the python-pin-dependencies branch from f879307 to 56c2c0d Compare March 4, 2020 17:27

Bobgy reviewed Mar 5, 2020

View reviewed changes

test/sample-test/requirements.in Show resolved Hide resolved

Add README to test/sample-test

15858ab

k8s-ci-robot added the lgtm label Mar 5, 2020

k8s-ci-robot assigned rmgogogo Mar 5, 2020

k8s-ci-robot added the approved label Mar 5, 2020

k8s-ci-robot merged commit 7e610d1 into kubeflow:master Mar 5, 2020

Bobgy mentioned this pull request Mar 6, 2020

[Engprod] Stable python package management #3078

Closed

Bobgy mentioned this pull request Mar 19, 2020

SDK - Added pinned dependency snapshot #3303

Merged

discordianfish mentioned this pull request Apr 2, 2020

backend/metadata_writer: Pin python dependencies #3408

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

backend: Pin python dependencies #3161

backend: Pin python dependencies #3161

discordianfish commented Feb 24, 2020 •

edited by jlewi

Loading

discordianfish commented Feb 24, 2020

Bobgy commented Feb 24, 2020

Bobgy Feb 24, 2020

discordianfish Feb 24, 2020

Bobgy Feb 24, 2020

Bobgy Feb 24, 2020

discordianfish commented Feb 25, 2020

discordianfish commented Feb 25, 2020

discordianfish commented Feb 25, 2020

Bobgy left a comment

Bobgy Feb 26, 2020

Bobgy commented Feb 26, 2020 •

edited

Loading

discordianfish commented Feb 26, 2020

discordianfish commented Feb 26, 2020

Bobgy left a comment

Bobgy Feb 26, 2020

discordianfish Feb 26, 2020

Bobgy Feb 26, 2020

discordianfish commented Mar 5, 2020

Bobgy commented Mar 5, 2020 •

edited

Loading

discordianfish commented Mar 5, 2020

jingzhang36 commented Mar 5, 2020

k8s-ci-robot commented Mar 5, 2020

backend: Pin python dependencies #3161

backend: Pin python dependencies #3161

Conversation

discordianfish commented Feb 24, 2020 • edited by jlewi Loading

discordianfish commented Feb 24, 2020

Bobgy commented Feb 24, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

discordianfish commented Feb 25, 2020

discordianfish commented Feb 25, 2020

discordianfish commented Feb 25, 2020

Bobgy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bobgy commented Feb 26, 2020 • edited Loading

discordianfish commented Feb 26, 2020

discordianfish commented Feb 26, 2020

Bobgy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

discordianfish commented Mar 5, 2020

Bobgy commented Mar 5, 2020 • edited Loading

discordianfish commented Mar 5, 2020

jingzhang36 commented Mar 5, 2020

k8s-ci-robot commented Mar 5, 2020

discordianfish commented Feb 24, 2020 •

edited by jlewi

Loading

Bobgy commented Feb 26, 2020 •

edited

Loading

Bobgy commented Mar 5, 2020 •

edited

Loading