[
https://issues.apache.org/jira/browse/BEAM-7746?focusedWorklogId=337763&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-337763
]
ASF GitHub Bot logged work on BEAM-7746:
----------------------------------------
Author: ASF GitHub Bot
Created on: 03/Nov/19 02:11
Start Date: 03/Nov/19 02:11
Worklog Time Spent: 10m
Work Description: chadrik commented on pull request #9056: [BEAM-7746]
Add python type hints
URL: https://github.com/apache/beam/pull/9056#discussion_r341832578
##########
File path: sdks/python/tox.ini
##########
@@ -160,16 +160,17 @@ commands =
[testenv:py27-lint]
# Checks for py2 syntax errors
deps =
+ -r build-requirements.txt
Review comment:
> tox runs outside of gradle. (At least for me). What is the issue there?
When tox is run on jenkins, it is invoked via gradle. The tox task is
defined in BeamModulePlugin.groovy like this:
```groovy
project.ext.toxTask = { name, tox_env ->
project.tasks.create(name) {
dependsOn 'setupVirtualenv'
dependsOn ':sdks:python:sdist'
doLast {
// Python source directory is also tox execution workspace, We
want
// to isolate them per tox suite to avoid conflict when running
// multiple tox suites in parallel.
project.copy { from project.pythonSdkDeps; into copiedSrcRoot }
def copiedPyRoot = "${copiedSrcRoot}/sdks/python"
def distTarBall = "${pythonRootDir}/build/apache-beam.tar.gz"
project.exec {
executable 'sh'
args '-c', ". ${project.ext.envdir}/bin/activate && cd
${copiedPyRoot} && scripts/run_tox.sh $tox_env $distTarBall"
}
}
inputs.files project.pythonSdkDeps
outputs.files project.fileTree(dir:
"${pythonRootDir}/target/.tox/${tox_env}/log/")
}
}
```
and it's used here:
https://github.com/apache/beam/blob/master/sdks/python/test-suites/tox/py2/build.gradle
> sdist step is used for other things (for building artifacts for release).
That is the reason for it to exists on its own.
I'm not saying that `sdist` task shouldn't exist, I'm saying it adds
overhead to use the `sdist` task to generate the tarball for the tox task,
instead of just letting tox generate its own sdist tarball, which it is
designed to do and is the default behavior (Also, there's the copying of source
files which adds more overhead and it's unclear why this exists since tox is
designed to create isolated virtualenvs per task environment).
In the definition of `toxTask` above, you see that it depends on
`setupVirtualenv` and also on `sdist`, and `sdist` itself depends on
`setupVirtualenv`. That's why we end up running `setupVirtualenv` twice for
each tox task run, and then tox creates yet another virtualenv to actually run
the task. That's 3 virtualenvs per tox job! I've used travisCI to setup a
number of tox-based CI jobs and the typical workflow is to ensure that tox is
installed *on the worker image* and then theres only one virtualenv created per
job, the one created by tox.
Anyway, long story short the `build-requirements.txt` line is required for
the tox tasks to work.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 337763)
Time Spent: 17h 10m (was: 17h)
> Add type hints to python code
> -----------------------------
>
> Key: BEAM-7746
> URL: https://issues.apache.org/jira/browse/BEAM-7746
> Project: Beam
> Issue Type: New Feature
> Components: sdk-py-core
> Reporter: Chad Dombrova
> Assignee: Chad Dombrova
> Priority: Major
> Time Spent: 17h 10m
> Remaining Estimate: 0h
>
> As a developer of the beam source code, I would like the code to use pep484
> type hints so that I can clearly see what types are required, get completion
> in my IDE, and enforce code correctness via a static analyzer like mypy.
> This may be considered a precursor to BEAM-7060
> Work has been started here: [https://github.com/apache/beam/pull/9056]
>
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
