You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2020/07/13 17:24:54 UTC

[GitHub] [beam] lazylynx opened a new pull request #12239: [BEAM-9980] tests tied with Python versions configurable

lazylynx opened a new pull request #12239:
URL: https://github.com/apache/beam/pull/12239


   Made it possible for some tests to configure which python version to use  with gradle properties
   
   ------------------------
   
   Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
   
    - [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
    - [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
    - [ ] Update `CHANGES.md` with noteworthy changes.
    - [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   
   See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
   
   Post-Commit Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   
   Lang | SDK | Dataflow | Flink | Samza | Spark | Twister2
   --- | --- | --- | --- | --- | --- | ---
   Go | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/) | ---
   Java | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
   Python | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/) | ---
   XLang | --- | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/) | ---
   
   Pre-Commit Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   
   --- |Java | Python | Go | Website
   --- | --- | --- | --- | ---
   Non-portable | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/)
   Portable | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/) | --- | ---
   
   See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] tvalentyn commented on a change in pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on a change in pull request #12239:
URL: https://github.com/apache/beam/pull/12239#discussion_r467346745



##########
File path: build.gradle
##########
@@ -220,13 +220,12 @@ task pythonPreCommit() {
   dependsOn ":sdks:python:test-suites:tox:py36:preCommitPy36"
   dependsOn ":sdks:python:test-suites:tox:py37:preCommitPy37"
   dependsOn ":sdks:python:test-suites:tox:py38:preCommitPy38"
-  dependsOn ":sdks:python:test-suites:dataflow:py2:preCommitIT"
-  dependsOn ":sdks:python:test-suites:dataflow:py2:preCommitIT_V2"
-  dependsOn ":sdks:python:test-suites:dataflow:py37:preCommitIT"
-  dependsOn ":sdks:python:test-suites:dataflow:py37:preCommitIT_V2"
-  // We don't include Py35, Py36 precommit ITs to reduce quota footprint.
-  // We can reconsider if we ever see an issue that these suites would
-  // have caught. Note that the same tests will still run in postcommit.
+  dependsOn ":sdks:python:test-suites:dataflow:preCommitIT"
+  dependsOn ":sdks:python:test-suites:dataflow:preCommitIT_V2"
+  // We don't include all supported Python versions precommit ITs to

Review comment:
       We can remove this comment. 

##########
File path: build.gradle
##########
@@ -248,9 +247,9 @@ task pythonFormatterPreCommit() {
 
 task python2PostCommit() {
   dependsOn ":sdks:python:test-suites:portable:py2:crossLanguagePythonJavaKafkaIOFlink"
-  dependsOn ":sdks:python:test-suites:portable:py2:crossLanguageTests"
+  dependsOn ":sdks:python:test-suites:portable:crossLanguageTests"

Review comment:
       I'm not sure I follow this change - this is currently still a python2 PostCommit suite, so it should include py2 only tests. 
   
   However you could convert the targets that are not version-specific, such as
   ```
   task portablePythonPreCommit() 
   task pythonSparkPostCommit() 
   ```
   to point to portable/build.gradle




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] tvalentyn commented on a change in pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on a change in pull request #12239:
URL: https://github.com/apache/beam/pull/12239#discussion_r466016946



##########
File path: sdks/python/test-suites/dataflow/build.gradle
##########
@@ -0,0 +1,50 @@
+/*

Review comment:
       It seems that we need to make changes to the top-level suite definitions, e.g. : https://github.com/apache/beam/blob/ba380270dbc8854ad44f6636a6767317a0416a99/build.gradle#L216
   to point to /test-suites/<runner>/build.gradle files.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] tvalentyn commented on a change in pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on a change in pull request #12239:
URL: https://github.com/apache/beam/pull/12239#discussion_r467352823



##########
File path: sdks/python/test-suites/gradle.properties
##########
@@ -0,0 +1,38 @@
+################################################################################
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+# limitations under the License.
+################################################################################
+
+# Following properties define which Python versions be used when the tests running.
+# Versions must be indicated major and minor versions connected with period.
+# And if you want to use multiple versions, concat them with comma and no whitespace.
+
+# dataflow test-suites
+dataflow_precommit_it_task_py_versions=2.7,3.7
+dataflow_mongodbio_it_task_py_versions=3.5
+dataflow_chicago_taxi_example_task_py_versions=2.7
+
+# direct test-suites
+hdfs_integration_test_task_py_versions=2.7,3.8
+direct_mongodbio_it_task_py_versions=2.7,3.5

Review comment:
       Let's modify https://github.com/apache/beam/blob/master/.test-infra/jenkins/job_PostCommit_Python_MongoDBIO_IT.groovy as well to point to direct/build.gradle.
   
   Also, it seems like there is a duplication in MongoDBIO suites. I think we can remove the separate jenkins Job for MongoDB IO and include it into the direct runner suites in a separate change.
   
   Also we can add version specifiers for MongoDB performance tests in another change.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] tvalentyn commented on a change in pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on a change in pull request #12239:
URL: https://github.com/apache/beam/pull/12239#discussion_r460351645



##########
File path: sdks/python/test-suites/gradle.properties
##########
@@ -0,0 +1,39 @@
+################################################################################
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+# limitations under the License.
+################################################################################
+
+# Following properties define which Python versions be used when the tests running.
+# Versions must be indicated major and minor versions connected with period.
+# And if you want to use multiple versions, concat them with comma and no whitespace.
+
+# dataflow test-suites

Review comment:
       This is a great reduction of  chaos in our test configurations! This would be super helpful for Python 2 deprecation.
   




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] lazylynx commented on pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
lazylynx commented on pull request #12239:
URL: https://github.com/apache/beam/pull/12239#issuecomment-670809891


   @tvalentyn updated. PTAL.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] tvalentyn commented on pull request #12239: [BEAM-9980] dataflow test-suite tasks tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on pull request #12239:
URL: https://github.com/apache/beam/pull/12239#issuecomment-675118006


   Run PythonDocker PreCommit


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] lazylynx commented on pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
lazylynx commented on pull request #12239:
URL: https://github.com/apache/beam/pull/12239#issuecomment-658199003


   R: @tvalentyn 


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] tvalentyn merged pull request #12239: [BEAM-9980] dataflow test-suite tasks tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
tvalentyn merged pull request #12239:
URL: https://github.com/apache/beam/pull/12239


   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] tvalentyn commented on pull request #12239: [BEAM-9980] dataflow test-suite tasks tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on pull request #12239:
URL: https://github.com/apache/beam/pull/12239#issuecomment-675131460


   Run PythonDocker PreCommit


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] tvalentyn edited a comment on pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
tvalentyn edited a comment on pull request #12239:
URL: https://github.com/apache/beam/pull/12239#issuecomment-663310669


   thanks a lot for this work, @lazylynx. Sorry for taking too long to respond, will try to take a closer look tomorrow.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] lazylynx commented on a change in pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
lazylynx commented on a change in pull request #12239:
URL: https://github.com/apache/beam/pull/12239#discussion_r469388011



##########
File path: sdks/python/test-suites/gradle.properties
##########
@@ -0,0 +1,38 @@
+################################################################################
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+# limitations under the License.
+################################################################################
+
+# Following properties define which Python versions be used when the tests running.
+# Versions must be indicated major and minor versions connected with period.
+# And if you want to use multiple versions, concat them with comma and no whitespace.
+
+# dataflow test-suites
+dataflow_precommit_it_task_py_versions=2.7,3.7
+dataflow_mongodbio_it_task_py_versions=3.5

Review comment:
       dataflow MongoDB suite is used here:
   https://github.com/apache/beam/blob/master/.test-infra/jenkins/job_PostCommit_Python_MongoDBIO_Load_Test.groovy#L54




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] tvalentyn commented on pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on pull request #12239:
URL: https://github.com/apache/beam/pull/12239#issuecomment-671689763


   Run Chicago Taxi on Dataflow


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] tvalentyn commented on a change in pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on a change in pull request #12239:
URL: https://github.com/apache/beam/pull/12239#discussion_r468298116



##########
File path: sdks/python/test-suites/gradle.properties
##########
@@ -0,0 +1,38 @@
+################################################################################
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+# limitations under the License.
+################################################################################
+
+# Following properties define which Python versions be used when the tests running.
+# Versions must be indicated major and minor versions connected with period.
+# And if you want to use multiple versions, concat them with comma and no whitespace.
+
+# dataflow test-suites
+dataflow_precommit_it_task_py_versions=2.7,3.7
+dataflow_mongodbio_it_task_py_versions=3.5
+dataflow_chicago_taxi_example_task_py_versions=2.7
+
+# direct test-suites
+hdfs_integration_test_task_py_versions=2.7,3.8
+direct_mongodbio_it_task_py_versions=2.7,3.5
+direct_runner_it_task_py_versions=2.7
+
+# portable test-suites
+cross_language_task_py_versions=2.7

Review comment:
       it looks like portable test suite configurations currently don't not have any effect, because the tasks from portable/build.gradle are not referenced in any groovy files or top-level tasks.  let's not add these configs and not add `/portable/build.gradle` in this change, and make those changes once we are ready to switch to using tasks from `portable/build.gradle`. 
   
   In current form, configs in `portable` section make an illusion that they control the versions of suites being used, but they currently don't. 




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] tvalentyn commented on pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on pull request #12239:
URL: https://github.com/apache/beam/pull/12239#issuecomment-671688946


   Run Python 3.6 PostCommit


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] lazylynx commented on a change in pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
lazylynx commented on a change in pull request #12239:
URL: https://github.com/apache/beam/pull/12239#discussion_r460507051



##########
File path: build.gradle
##########
@@ -243,13 +241,16 @@ task pythonFormatterPreCommit() {
   dependsOn 'sdks:python:test-suites:tox:py38:formatter'
 }
 
+task pythonPostCommit() {

Review comment:
       This task `pythonPostCommit` aims to trigger tasks whose python versions defined in gradle.properties, which should not to be triggered by PostCommit tasks with versions like `python2PostCommit`.
   We must reconsider this task configuration when introducing high/low priority.
   
   Did I answer your comment? 




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] tvalentyn commented on pull request #12239: [BEAM-9980] dataflow test-suite tasks tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on pull request #12239:
URL: https://github.com/apache/beam/pull/12239#issuecomment-675115938


   Run Chicago Taxi on Dataflow


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] tvalentyn commented on pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on pull request #12239:
URL: https://github.com/apache/beam/pull/12239#issuecomment-671692603


   Run Python 2 PostCommit


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] lazylynx commented on a change in pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
lazylynx commented on a change in pull request #12239:
URL: https://github.com/apache/beam/pull/12239#discussion_r464536071



##########
File path: sdks/python/test-suites/dataflow/build.gradle
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * License); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an AS IS BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+apply plugin: org.apache.beam.gradle.BeamModulePlugin
+applyPythonNature()
+
+dependencies {
+  distTarBall project(path: ":sdks:python", configuration: "distTarBall")
+}
+
+task preCommitIT {
+  getListProperty('dataflow_precommit_it_task_py_versions').each {

Review comment:
       Quite so. I use `getVersionsAsList` instead of `getListProperty`.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] lazylynx commented on a change in pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
lazylynx commented on a change in pull request #12239:
URL: https://github.com/apache/beam/pull/12239#discussion_r464534655



##########
File path: sdks/python/test-suites/dataflow/py37/build.gradle
##########
@@ -20,10 +20,6 @@ apply plugin: org.apache.beam.gradle.BeamModulePlugin
 applyPythonNature()
 enablePythonPerformanceTest()

Review comment:
       Moved.
   And `addPortableWordCountTasks()` in portable test-suites is also moved to common.gradle, which is invoked at every build.gradle under py* dirs in the test-suites.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] tvalentyn commented on a change in pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on a change in pull request #12239:
URL: https://github.com/apache/beam/pull/12239#discussion_r469406587



##########
File path: sdks/python/test-suites/gradle.properties
##########
@@ -0,0 +1,38 @@
+################################################################################
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+# limitations under the License.
+################################################################################
+
+# Following properties define which Python versions be used when the tests running.
+# Versions must be indicated major and minor versions connected with period.
+# And if you want to use multiple versions, concat them with comma and no whitespace.
+
+# dataflow test-suites
+dataflow_precommit_it_task_py_versions=2.7,3.7
+dataflow_mongodbio_it_task_py_versions=3.5

Review comment:
       I see, thanks for correction. Then we could keep this setting, but, add it together with the changes in Groovy file.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] tvalentyn commented on a change in pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on a change in pull request #12239:
URL: https://github.com/apache/beam/pull/12239#discussion_r468290542



##########
File path: sdks/python/test-suites/gradle.properties
##########
@@ -0,0 +1,38 @@
+################################################################################
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+# limitations under the License.
+################################################################################
+
+# Following properties define which Python versions be used when the tests running.
+# Versions must be indicated major and minor versions connected with period.
+# And if you want to use multiple versions, concat them with comma and no whitespace.
+
+# dataflow test-suites
+dataflow_precommit_it_task_py_versions=2.7,3.7
+dataflow_mongodbio_it_task_py_versions=3.5
+dataflow_chicago_taxi_example_task_py_versions=2.7
+
+# direct test-suites
+hdfs_integration_test_task_py_versions=2.7,3.8
+direct_mongodbio_it_task_py_versions=2.7,3.5

Review comment:
       I think MongoDB IO can run with Direct runner postcommit, similar to HDFS IO. job_PostCommit_Python_MongoDBIO_Load_Test can be a separate job, for which we control the version in settings.gradle.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] tvalentyn commented on pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on pull request #12239:
URL: https://github.com/apache/beam/pull/12239#issuecomment-671688460


   Thanks, @lazylynx . The change LGTM, will run some tests to make sure things didn't break.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] tvalentyn commented on a change in pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on a change in pull request #12239:
URL: https://github.com/apache/beam/pull/12239#discussion_r468301103



##########
File path: sdks/python/test-suites/gradle.properties
##########
@@ -0,0 +1,38 @@
+################################################################################
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+# limitations under the License.
+################################################################################
+
+# Following properties define which Python versions be used when the tests running.
+# Versions must be indicated major and minor versions connected with period.
+# And if you want to use multiple versions, concat them with comma and no whitespace.
+
+# dataflow test-suites
+dataflow_precommit_it_task_py_versions=2.7,3.7
+dataflow_mongodbio_it_task_py_versions=3.5

Review comment:
       I don't think we have a dataflow MongoDB suite - I only see a direct runner suite.

##########
File path: sdks/python/test-suites/gradle.properties
##########
@@ -0,0 +1,38 @@
+################################################################################
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+# limitations under the License.
+################################################################################
+
+# Following properties define which Python versions be used when the tests running.
+# Versions must be indicated major and minor versions connected with period.
+# And if you want to use multiple versions, concat them with comma and no whitespace.
+
+# dataflow test-suites
+dataflow_precommit_it_task_py_versions=2.7,3.7
+dataflow_mongodbio_it_task_py_versions=3.5
+dataflow_chicago_taxi_example_task_py_versions=2.7
+
+# direct test-suites
+hdfs_integration_test_task_py_versions=2.7,3.8
+direct_mongodbio_it_task_py_versions=2.7,3.5
+direct_runner_it_task_py_versions=2.7
+
+# portable test-suites
+cross_language_task_py_versions=2.7

Review comment:
       one more comment: it looks like portable test suite configuration currently does not have any effect, because the tasks from portable/build.gradle are not referenced in any groovy files or top-level tasks.  let's not add these configs and not add `/portable/build.gradle` in this change, and make those changes once we are ready to switch to using tasks from `portable/build.gradle`. 
   
   In current form, configs in `portable` section make an illusion that they control the versions of suites being used, but they currently don't. 




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] lazylynx commented on a change in pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
lazylynx commented on a change in pull request #12239:
URL: https://github.com/apache/beam/pull/12239#discussion_r467879685



##########
File path: sdks/python/test-suites/gradle.properties
##########
@@ -0,0 +1,38 @@
+################################################################################
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+# limitations under the License.
+################################################################################
+
+# Following properties define which Python versions be used when the tests running.
+# Versions must be indicated major and minor versions connected with period.
+# And if you want to use multiple versions, concat them with comma and no whitespace.
+
+# dataflow test-suites
+dataflow_precommit_it_task_py_versions=2.7,3.7
+dataflow_mongodbio_it_task_py_versions=3.5
+dataflow_chicago_taxi_example_task_py_versions=2.7
+
+# direct test-suites
+hdfs_integration_test_task_py_versions=2.7,3.8
+direct_mongodbio_it_task_py_versions=2.7,3.5

Review comment:
       Updated `beam_PostCommit_Python_MongoDBIO_IT.groovy`
   
   > I think we can remove the separate jenkins Job for MongoDB IO and include it into the direct runner suites in a separate change.
   
   You mean `job_PostCommit_Python_MongoDBIO_IT` and `job_PostCommit_Python_MongoDBIO_Load_Test` are to be moved to direct runner suites? @tvalentyn 




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] tvalentyn commented on pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on pull request #12239:
URL: https://github.com/apache/beam/pull/12239#issuecomment-671688647


   Run Python MongoDBIO_IT
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] tvalentyn commented on a change in pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on a change in pull request #12239:
URL: https://github.com/apache/beam/pull/12239#discussion_r466015279



##########
File path: sdks/python/test-suites/dataflow/build.gradle
##########
@@ -0,0 +1,50 @@
+/*

Review comment:
       Nevermind, I see that they both serve a different purpose.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] lazylynx commented on a change in pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
lazylynx commented on a change in pull request #12239:
URL: https://github.com/apache/beam/pull/12239#discussion_r467349799



##########
File path: build.gradle
##########
@@ -220,13 +220,12 @@ task pythonPreCommit() {
   dependsOn ":sdks:python:test-suites:tox:py36:preCommitPy36"
   dependsOn ":sdks:python:test-suites:tox:py37:preCommitPy37"
   dependsOn ":sdks:python:test-suites:tox:py38:preCommitPy38"
-  dependsOn ":sdks:python:test-suites:dataflow:py2:preCommitIT"
-  dependsOn ":sdks:python:test-suites:dataflow:py2:preCommitIT_V2"
-  dependsOn ":sdks:python:test-suites:dataflow:py37:preCommitIT"
-  dependsOn ":sdks:python:test-suites:dataflow:py37:preCommitIT_V2"
-  // We don't include Py35, Py36 precommit ITs to reduce quota footprint.
-  // We can reconsider if we ever see an issue that these suites would
-  // have caught. Note that the same tests will still run in postcommit.
+  dependsOn ":sdks:python:test-suites:dataflow:preCommitIT"
+  dependsOn ":sdks:python:test-suites:dataflow:preCommitIT_V2"
+  // We don't include all supported Python versions precommit ITs to

Review comment:
       OK, removed.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] tvalentyn commented on a change in pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on a change in pull request #12239:
URL: https://github.com/apache/beam/pull/12239#discussion_r468303799



##########
File path: sdks/python/test-suites/gradle.properties
##########
@@ -0,0 +1,38 @@
+################################################################################
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+# limitations under the License.
+################################################################################
+
+# Following properties define which Python versions be used when the tests running.
+# Versions must be indicated major and minor versions connected with period.
+# And if you want to use multiple versions, concat them with comma and no whitespace.
+
+# dataflow test-suites
+dataflow_precommit_it_task_py_versions=2.7,3.7
+dataflow_mongodbio_it_task_py_versions=3.5
+dataflow_chicago_taxi_example_task_py_versions=2.7

Review comment:
       Does this configuration have any effect?




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] tvalentyn commented on a change in pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on a change in pull request #12239:
URL: https://github.com/apache/beam/pull/12239#discussion_r467352823



##########
File path: sdks/python/test-suites/gradle.properties
##########
@@ -0,0 +1,38 @@
+################################################################################
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+# limitations under the License.
+################################################################################
+
+# Following properties define which Python versions be used when the tests running.
+# Versions must be indicated major and minor versions connected with period.
+# And if you want to use multiple versions, concat them with comma and no whitespace.
+
+# dataflow test-suites
+dataflow_precommit_it_task_py_versions=2.7,3.7
+dataflow_mongodbio_it_task_py_versions=3.5
+dataflow_chicago_taxi_example_task_py_versions=2.7
+
+# direct test-suites
+hdfs_integration_test_task_py_versions=2.7,3.8
+direct_mongodbio_it_task_py_versions=2.7,3.5

Review comment:
       Let's modify https://github.com/apache/beam/blob/master/.test-infra/jenkins/job_PostCommit_Python_MongoDBIO_IT.groovy for now.
   
   Also, it seems like there is a duplication in MongoDBIO suites. I think we can remove the separate jenkins Job for MongoDB IO and include it into the direct runner suites in a separate change.
   
   Also we can add version specifiers for MongoDB performance tests in another change.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] lazylynx commented on pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
lazylynx commented on pull request #12239:
URL: https://github.com/apache/beam/pull/12239#issuecomment-672982264


   @tvalentyn OK, I will split this PR to be separated by test-suites.
   And this PR will be updated to one for dataflow test-suite. 


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] tvalentyn commented on pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on pull request #12239:
URL: https://github.com/apache/beam/pull/12239#issuecomment-671688813


   Run Python 3.5 PostCommit


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] lazylynx commented on pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
lazylynx commented on pull request #12239:
URL: https://github.com/apache/beam/pull/12239#issuecomment-663968363


   Thank you for review @tvalentyn !
   I prefer to split to separated commits, I will update and resolve conflicts later.
   
   And thank you for suggestion for introducing the concept of high/low priority versions.
   I do agree your plan.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] tvalentyn commented on pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on pull request #12239:
URL: https://github.com/apache/beam/pull/12239#issuecomment-663310669


   thanks a lot for this work, @lazylynx. Sorry for taking too long to respond, will try to take a look tomorrow.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] tvalentyn commented on a change in pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on a change in pull request #12239:
URL: https://github.com/apache/beam/pull/12239#discussion_r463910737



##########
File path: build.gradle
##########
@@ -243,13 +241,16 @@ task pythonFormatterPreCommit() {
   dependsOn 'sdks:python:test-suites:tox:py38:formatter'
 }
 
+task pythonPostCommit() {

Review comment:
       I think adding a new Jenkins job in this PR might add more confusion until we clarify how to reconcile this suite with other postcommit suites. 
   
   How about we keep the test suites in the jenkins jobs where they are defined right now, but make the python versions configurable in the gradle.properties file? This would allow us to merge most of these changes faster, including the refactoring to use suites definitions from common.gradle.
   
   Later we can reconsider the shapes of Jenkins job when we define High Priortity/Low priority versions.

##########
File path: sdks/python/test-suites/dataflow/py37/build.gradle
##########
@@ -20,10 +20,6 @@ apply plugin: org.apache.beam.gradle.BeamModulePlugin
 applyPythonNature()
 enablePythonPerformanceTest()

Review comment:
       Can `enablePythonPerformanceTest()` be moved to common.gradle as well?

##########
File path: .test-infra/jenkins/job_PostCommit_Python_MongoDBIO_Load_Test.groovy
##########
@@ -51,7 +51,7 @@ job(jobName) {
       rootBuildScriptDir(common.checkoutDir)
       common.setGradleSwitches(delegate)
       switches("-Popts=\'${common.mapToArgString(pipelineOptions)}\'")
-      tasks(":sdks:python:test-suites:dataflow:py35:mongodbioIT")
+      tasks(":sdks:python:test-suites:dataflow:mongodbioIT")

Review comment:
       Note that this is a load test, and we also have integration test suite: https://github.com/apache/beam/blob/d971ba13b8ed31de3413558f701d752cbf202d47/.test-infra/jenkins/job_PostCommit_Python_MongoDBIO_IT.groovy#L35

##########
File path: sdks/python/test-suites/dataflow/build.gradle
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * License); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an AS IS BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+apply plugin: org.apache.beam.gradle.BeamModulePlugin
+applyPythonNature()
+
+dependencies {
+  distTarBall project(path: ":sdks:python", configuration: "distTarBall")
+}
+
+task preCommitIT {
+  getListProperty('dataflow_precommit_it_task_py_versions').each {

Review comment:
       I think for a reader `getListProperty` name might be confusing  and they would have to look up the implementation. We could consider the name `getVersionsAsList` or use `getProperty('dataflow_precommit_it_task_py_versions').split(',').each` directly.

##########
File path: build.gradle
##########
@@ -243,13 +241,16 @@ task pythonFormatterPreCommit() {
   dependsOn 'sdks:python:test-suites:tox:py38:formatter'
 }
 
+task pythonPostCommit() {

Review comment:
       (missed this comment) I think I understand what you mean.
   Let's consider a contributor who is adding a new test or test suite that needs to run in postcommit (after PR is merged). How would they configure their suite? Looking at this file it may not be obvious, for example hdfs test suite is added in PostCommit and Py38 PostCommit but MongoDBIO suite is only defined in PostCommit. 
   How can we make the configuration more straightforward?
   We should also consider what guidance we can give contributors when to create a new jenkins job and when to add to add to existing postcommit jobs. For example we have a separate job for MongoDB IO, but do not for HDFS, probably because both tests were added by different people and the configuration was not easy to figure out.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] tvalentyn commented on pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on pull request #12239:
URL: https://github.com/apache/beam/pull/12239#issuecomment-671689948


   Run Python MongoDBIO_IT


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] lazylynx commented on a change in pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
lazylynx commented on a change in pull request #12239:
URL: https://github.com/apache/beam/pull/12239#discussion_r466531826



##########
File path: sdks/python/test-suites/dataflow/build.gradle
##########
@@ -0,0 +1,50 @@
+/*

Review comment:
       I've rollbacked too much.
   Made top level build.gradle tasks depend on test-suites ones AMAP.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] lazylynx commented on a change in pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
lazylynx commented on a change in pull request #12239:
URL: https://github.com/apache/beam/pull/12239#discussion_r467349765



##########
File path: build.gradle
##########
@@ -248,9 +247,9 @@ task pythonFormatterPreCommit() {
 
 task python2PostCommit() {
   dependsOn ":sdks:python:test-suites:portable:py2:crossLanguagePythonJavaKafkaIOFlink"
-  dependsOn ":sdks:python:test-suites:portable:py2:crossLanguageTests"
+  dependsOn ":sdks:python:test-suites:portable:crossLanguageTests"

Review comment:
       restored crossLanguageTests and directRunnerIT to py2 only.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] tvalentyn commented on pull request #12239: [BEAM-9980] dataflow test-suite tasks tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on pull request #12239:
URL: https://github.com/apache/beam/pull/12239#issuecomment-675137948


   MongoDB failure is unrelated, opened https://issues.apache.org/jira/browse/BEAM-10718.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] tvalentyn commented on pull request #12239: [BEAM-9980] dataflow test-suite tasks tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on pull request #12239:
URL: https://github.com/apache/beam/pull/12239#issuecomment-675116946


   Run Python MongoDBIO Load Test


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] tvalentyn commented on a change in pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on a change in pull request #12239:
URL: https://github.com/apache/beam/pull/12239#discussion_r466012301



##########
File path: sdks/python/test-suites/dataflow/build.gradle
##########
@@ -0,0 +1,50 @@
+/*

Review comment:
       why do we need build.gradle in addition to common.grade?




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] tvalentyn commented on pull request #12239: [BEAM-9980] dataflow test-suite tasks tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on pull request #12239:
URL: https://github.com/apache/beam/pull/12239#issuecomment-675116187


   Run Python MongoDBIO_IT


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] tvalentyn commented on a change in pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on a change in pull request #12239:
URL: https://github.com/apache/beam/pull/12239#discussion_r468304216



##########
File path: sdks/python/test-suites/direct/build.gradle
##########
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * License); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an AS IS BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+plugins { id 'org.apache.beam.module' }
+applyPythonNature()
+
+task directRunnerIT {

Review comment:
       This task appears to be unused.

##########
File path: sdks/python/test-suites/gradle.properties
##########
@@ -0,0 +1,38 @@
+################################################################################
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+# limitations under the License.
+################################################################################
+
+# Following properties define which Python versions be used when the tests running.
+# Versions must be indicated major and minor versions connected with period.
+# And if you want to use multiple versions, concat them with comma and no whitespace.
+
+# dataflow test-suites
+dataflow_precommit_it_task_py_versions=2.7,3.7
+dataflow_mongodbio_it_task_py_versions=3.5
+dataflow_chicago_taxi_example_task_py_versions=2.7
+
+# direct test-suites
+hdfs_integration_test_task_py_versions=2.7,3.8

Review comment:
       Does this configuration have any effect?

##########
File path: sdks/python/test-suites/direct/build.gradle
##########
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * License); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an AS IS BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+plugins { id 'org.apache.beam.module' }
+applyPythonNature()
+
+task directRunnerIT {
+  getVersionsAsList('direct_runner_it_task_py_versions').each {
+    dependsOn.add(":sdks:python:test-suites:direct:py${getVersionSuffix(it)}:directRunnerIT")
+  }
+}
+
+task hdfsIntegrationTest {

Review comment:
       This task appears to be unused.

##########
File path: sdks/python/test-suites/direct/build.gradle
##########
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * License); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an AS IS BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+plugins { id 'org.apache.beam.module' }
+applyPythonNature()
+
+task directRunnerIT {
+  getVersionsAsList('direct_runner_it_task_py_versions').each {
+    dependsOn.add(":sdks:python:test-suites:direct:py${getVersionSuffix(it)}:directRunnerIT")
+  }
+}
+
+task hdfsIntegrationTest {

Review comment:
       Please also take a look at other build.gradle files, let's only add new tasks when we connect them with top-level test tasks or groovy files that trigger test suites. 

##########
File path: sdks/python/test-suites/dataflow/build.gradle
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * License); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an AS IS BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+apply plugin: org.apache.beam.gradle.BeamModulePlugin
+applyPythonNature()
+
+dependencies {
+  distTarBall project(path: ":sdks:python", configuration: "distTarBall")
+}
+
+task preCommitIT {
+  getVersionsAsList('dataflow_precommit_it_task_py_versions').each {
+    dependsOn.add(":sdks:python:test-suites:dataflow:py${getVersionSuffix(it)}:preCommitIT_batch")
+    dependsOn.add(":sdks:python:test-suites:dataflow:py${getVersionSuffix(it)}:preCommitIT_streaming")
+  }
+}
+
+task preCommitIT_V2{
+  getVersionsAsList('dataflow_precommit_it_task_py_versions').each {
+    dependsOn.add(":sdks:python:test-suites:dataflow:py${getVersionSuffix(it)}:preCommitIT_batch_V2")
+    dependsOn.add(":sdks:python:test-suites:dataflow:py${getVersionSuffix(it)}:preCommitIT_streaming_V2")
+  }
+}
+
+task mongodbioIT {
+  getVersionsAsList('dataflow_mongodbio_it_task_py_versions').each {
+    dependsOn.add(":sdks:python:test-suites:dataflow:py${getVersionSuffix(it)}:mongodbioIT")
+  }
+}
+
+task chicagoTaxiExample {

Review comment:
       This task appears to be unused




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] tvalentyn commented on a change in pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on a change in pull request #12239:
URL: https://github.com/apache/beam/pull/12239#discussion_r458295913



##########
File path: build.gradle
##########
@@ -243,13 +241,16 @@ task pythonFormatterPreCommit() {
   dependsOn 'sdks:python:test-suites:tox:py38:formatter'
 }
 
+task pythonPostCommit() {

Review comment:
       I am not sure I understand which suites we include in this postcommit suite and how we select which version are high priority.

##########
File path: sdks/python/test-suites/gradle.properties
##########
@@ -0,0 +1,39 @@
+################################################################################
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+# limitations under the License.
+################################################################################
+
+# Following properties define which Python versions be used when the tests running.
+# Versions must be indicated major and minor versions connected with period.
+# And if you want to use multiple versions, concat them with comma and no whitespace.
+
+# dataflow test-suites

Review comment:
       This is a great chaos reduction in our test suite! This would be super helpful for Python 2 deprecation.
   




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] lazylynx commented on pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
lazylynx commented on pull request #12239:
URL: https://github.com/apache/beam/pull/12239#issuecomment-668129012


   @tvalentyn Thank you for detail comments.
   I made codes to just refactor current tests. PTAL.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] tvalentyn commented on a change in pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on a change in pull request #12239:
URL: https://github.com/apache/beam/pull/12239#discussion_r469406587



##########
File path: sdks/python/test-suites/gradle.properties
##########
@@ -0,0 +1,38 @@
+################################################################################
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+# limitations under the License.
+################################################################################
+
+# Following properties define which Python versions be used when the tests running.
+# Versions must be indicated major and minor versions connected with period.
+# And if you want to use multiple versions, concat them with comma and no whitespace.
+
+# dataflow test-suites
+dataflow_precommit_it_task_py_versions=2.7,3.7
+dataflow_mongodbio_it_task_py_versions=3.5

Review comment:
       I see, thanks for correction. Then we could keep this setting, but, modify add it together with the Groovy file.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [beam] lazylynx commented on a change in pull request #12239: [BEAM-9980] tests tied with Python versions configurable

Posted by GitBox <gi...@apache.org>.
lazylynx commented on a change in pull request #12239:
URL: https://github.com/apache/beam/pull/12239#discussion_r464532005



##########
File path: build.gradle
##########
@@ -243,13 +241,16 @@ task pythonFormatterPreCommit() {
   dependsOn 'sdks:python:test-suites:tox:py38:formatter'
 }
 
+task pythonPostCommit() {

Review comment:
       Thank you for detail description.
   It did not occur to me.




----------------------------------------------------------------
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:
users@infra.apache.org