Smoke test #346
Smoke test #346
Conversation
|
In the circle ci test below, there are binary_linux_conda_, binary_macos_wheel_. Could we add a |
|
not sure about the "having to regenerate the binaries" part. we should only build once, and if it's nightlies, we upload output, then run smoke test. |
…ker image which can make nightlies installed via pip pass
README.md
Outdated
|
|
||
| ``` | ||
| pip install numpy | ||
| pip install future # only on python2.7 |
There was a problem hiding this comment.
nit: space missing: python 2.7
| command: | | ||
| set -x | ||
| source /usr/local/etc/profile.d/conda.sh && conda activate python${PYTHON_VERSION} | ||
| conda install -y -c pytorch-nightly torchaudio |
There was a problem hiding this comment.
The PyTorch smoketests fix the version date, so they fail if the nightlies failed. Should we do that here? (I'm indifferent, just pointing out the discrepancy)
There was a problem hiding this comment.
Another option is to have the smoke test not run if the nightlies fail? Or do you have another suggestion?
There was a problem hiding this comment.
I'm not 100% sure how conda will resolve target versions, if it picks the latest version, then test here might still make sense. I'll spend sometime looking into this later
| docker_build: | ||
| triggers: | ||
| - schedule: | ||
| cron: "0 10 * * 0" |
There was a problem hiding this comment.
Reminder that cron jobs don't get access to environment variables in secret contexts. If you want to do jobs with access to them, you have to do the Chronos triggered push trick
There was a problem hiding this comment.
looks like circleCI fix this:
https://circleci.com/workflow-run/7dc8be6a-9d61-4b14-b6cb-7394f32245f1
the docker build job is triggered by cron and credentials got passed in
There was a problem hiding this comment.
No, I disagree with your assessment of the job. Look at https://app.circleci.com/jobs/github/pytorch/pytorch/3712495 and expand "Spin up Environment". None of the environment variables mentioned are those from the context https://circleci.com/gh/organizations/pytorch/settings#contexts/2b885fc9-ef3a-4b86-8f5a-2e6e22bd0cfe
Maybe these smoketests don't actually need the secrets from the context, which is why everything is OK. But just FYI.
There was a problem hiding this comment.
OK, so what I used is environment variable, which is define here: https://circleci.com/gh/pytorch/audio/edit#env-vars , it's different than context
|
|
||
| ``` | ||
| pip install numpy | ||
| pip install future # only on python 2.7 | ||
| pip install six # only on python 3.5 |
There was a problem hiding this comment.
I'm surprised we have to add the dependencies explicitly :(
There was a problem hiding this comment.
not sure why pip didn't do that, my tests showed I have to install them manually.
.circleci/docker/Dockerfile
Outdated
|
|
||
| FROM ubuntu:latest | ||
|
|
||
| RUN apt-get -qq update && apt-get -qq -y install curl bzip2 \ |
There was a problem hiding this comment.
Consider putting this in a shell script?
There was a problem hiding this comment.
I'd prefer to use current format since there are just list of commands, no complex logic, still very readable to me.
.circleci/docker/Dockerfile
Outdated
|
|
||
| ENV PATH /opt/conda/bin:$PATH | ||
|
|
||
| RUN apt update |
There was a problem hiding this comment.
I don't think you want apt update as its own RUN, as this will create a layer that has all of the apt metadata included (but generally we want to make sure this data doesn't show up in the docker image, as it's just bloat and thwarts caching)
|
As a possible future smoke test, it'd be good to verify that the tutorial can run. |
no, new docker from scratch |
Adding smoke test after nightly build and upload. Currently the scope is limited to Linux. The test environment is a docker container that has all Python versions we need with dependencies installed.
This PR also added docker image builder, which will build a new image every week. But we need to manually pick one for smoke test.
we only test "import torchaudio" at the moment.