To follow up, since I'm trying to run cython-based tests using pytest: - tox does in fact correctly install apache-beam with cythonized modules in its virtualenv. - Since our tests are under apache_beam/, local sources shadow those in the installed apache_beam package. - The original issue I raised (BEAM-8572 <https://issues.apache.org/jira/browse/BEAM-8572>) was due to bad usage of utils.check_compiled().
So I'm going to add an additional "python setup.py build_ext --inplace" to pyXX-cython-pytest envs. If we want to remove this additional step we'll probably have to move tests under a separate directory that is not part of the apache_beam package. See also: https://github.com/tox-dev/tox/issues/514 https://blog.ionelmc.ro/2014/05/25/python-packaging/#the-structure On Mon, Nov 11, 2019 at 3:21 PM Ahmet Altay <al...@google.com> wrote: > Thank you for spending time on this to clarify it for all of us! Much > appreciated. > > On Sun, Nov 10, 2019 at 3:45 PM Chad Dombrova <chad...@gmail.com> wrote: > >> Hi all, >> >> >>> The sdist step creates a package that should be installed into each >>> tox environment. If the tox environment has cython when this apache >>> beam package is installed, it should be used. Nose (or whatever) >>> should then run the tests. >>> >> I spent some time this weekend trying to understand the Beam python build >> process, and here’s an overview of what I’ve learned: >> >> - the :sdks:python:sdist gradle task creates the source tarball (no >> surprises there) >> - the protobuf stubs are generated during this process >> - the sdist is provided to tox, which installs it into the the >> virtualenv for that task >> - for *-cython tasks, tox installs the cython dep and, as Ahmet >> asserted, python setup.py nosetests performs the cythonization. >> - this cythonized build overrides the one installed by tox >> >> Here’s what I learned about the current status of tests wrt cython: >> >> - cython tox tasks *are* using cython (good!) >> - non-cython tox tasks *are not* using cython (good!) >> - none of the GCP or integration tests are using cython (bad?) >> >> This is intentional with the recent change to drop base only tests. > Otherwise we would not have coverage for non-cythonized tests. > >> >> - This is because the build is only cythonized when python setup.py >> nosetests is used in conjunction with tox (tox installs cython, python >> setup.py nosetests compiles it). >> - GCP tests don't install cython. ITs don't use tox. >> >> To confirm my understanding of this, I created a PR [1] to assert that a >> cythonized or pure-python build is being used. A cythonized build is >> expected by default on linux systems unless a special flag is provided to >> inform the test otherwise. It appears as though the portable tests passed >> (i.e. used cython), but I forgot to add the assertion for those; like the >> other ITs they are not using cython. >> >> *Questions:* >> >> - Is the lack of cython for ITs expected and/or desired? >> - Why aren't ITs using tox? It's quite possible to pass arguments >> into tox to control it's behavior. For example, it seems reasonable that >> run_integration_test.sh could be inside tox >> >> > For ITs the primary execution will happen on a remote system. As long as > those remote workers have cython installed the tests will run in a > cythonized environment. This is true for any IT tests, that runs in a > container based on the Dockerfile we have in beam (which installs cython), > and also true for legacy Dataflow environments that are not yet using this > Beam provided Dockerfile. > > >> >> *Next Steps:*There has been some movement in the python community to >> solve problems around build dependencies [2] and toolchains [3]. I hope to >> have a proposal for how to simplify this process soon. >> > Thank you! > > [1] https://github.com/apache/beam/pull/10058 >> [2] https://www.python.org/dev/peps/pep-0517/ >> [3] https://www.python.org/dev/peps/pep-0518/ >> >> -chad >> >
smime.p7s
Description: S/MIME Cryptographic Signature
