Updates qemu framework to install pip requirements from its own requirements file#14355
Conversation
|
Very interesting findings. |
|
@marcoabreu could you please merge this? ^^ |
|
I don't agree with the approach because it removes transparency and instead makes the project rely on a provided blob (aka the OS image used in the Emulator). Pedro chose this approach specifically to keep up reproducibility. From what I can tell, we are trying to circumvent a different problem here, which is that windows as well as qemu use the same test requirements. So I'd either propose to generate a new OS image (preferred solution) or split the requirements. |
|
I agree that this is the preferred solution. But, a few things to keep in mind:
|
|
Good points @perdasilva |
|
For the sake of maintaining the mental model for future eyeballs to look at. Leaving breadcrumbs in expected places with the right information to show the err of this way makes for a cleaner knowledge trail. Though it is not pragmatic... it is keeping good hygiene for the future.
|
d10a9cf to
ec5f6a0
Compare
|
@gavinmbell @marcoabreu I've changed the approach slightly. Is this more agreeable? re. "From what I can tell, we are trying to circumvent a different problem here, which is that windows as well as qemu use the same test requirements." - actually, some of the tests require numpy and scipy. This issue just wasnt caught before because the test requirements are also installed in the the linux docker environments in install/ubuntu_python.sh (which in this case include scipy). I guess, previously, the windows ami also pip installed these dependencies, which is why it wasnt caught earlier. Idk. |
ci/qemu/README.md
Outdated
|
|
||
| # CI and Testing | ||
|
|
||
| Formally, [runtime_functions.py](https://github.com/apache/incubator-mxnet/blob/master/ci/docker/qemu/runtime_functions.py) would [run](https://github.com/apache/incubator-mxnet/blob/8beea18e3d9835f90b59d3f9de8f9945ac819423/ci/docker/qemu/runtime_functions.py#L81) *pip install -r [mxnet/tests/requirements.txt](https://github.com/apache/incubator-mxnet/blob/master/tests/requirements.txt)*. If the requirements change, there can be an unfortunate side-effect that there are no wheel files for Raspberry Pi for the new requirement. This would trigger a build from source on the emulator, which can take a long time and cause job timeouts. Therefore, we no longer install the `tests/requirements.txt` requirements, but rather rely on [mxnet-requirements.txt](https://github.com/apache/incubator-mxnet/blob/master/ci/qemu/mxnet_requirements.txt) to maintain the requirements for the qemu tests. Should any requirements changes lead to a job time out, it is incumbent on the submitter to update the image to include the requirement and unblock ci. |
There was a problem hiding this comment.
Good... So at least there is a pointer for folks to know what does NOT work!
👍
|
Nit: You might wanna adapt the title. |
|
Lets put this to bed Mr nit :-) |
ec5f6a0 to
5fa2b8a
Compare
5fa2b8a to
8633a51
Compare
|
@mxnet-label-bot add [CI, pr-awaiting-review] @perdasilva could you have a look at the CI failure? |
8633a51 to
9949e5b
Compare
9949e5b to
02200cd
Compare
* Backports Windows CI pipeline fixes from master * Updates Jenkins file to use python 3 when calling build_windows.py * Installs qemu pip requirements from qemu requirements file (#14355) * remove a white space
Description
Removes the installation of tests/requirements.txt. If this file contains requirements for which there are no existing Raspberry Pi wheel files, a build from source will be triggered. This can cause CI timeouts. From now on, the python requirements for qemu tests should to be managed separately.
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.