You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Michael Smith (Code Review)" <ge...@cloudera.org> on 2022/06/02 22:46:02 UTC

[Impala-ASF-CR] Python3 testing

Michael Smith has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18586


Change subject: Python3 testing
......................................................................

Python3 testing

Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
---
M CMakeLists.txt
M bin/impala-shell.sh
M shell/impala-shell
M shell/make_shell_tarball.sh
D shell/pkg_resources.py
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
M tests/shell/util.py
8 files changed, 11 insertions(+), 2,706 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/86/18586/1
-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................


Patch Set 8:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10707/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 8
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 06 Jun 2022 16:42:28 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18586/5/shell/CMakeLists.txt
File shell/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/18586/5/shell/CMakeLists.txt@39
PS5, Line 39: shell/build/dist/
Keep consistent with the path in shell/packaging/make_python_package.sh



-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 03 Jun 2022 17:39:45 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18586

to look at the new patch set (#5).

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................

IMPALA-11314: Test PyPI package with system python

Sets up a virtualenv with system python to install the impala-shell PyPI
package into. Using system python provides better coverage for Python
versions likely to be used by customers. Runs impala-shell tests using
the PyPI package to provide better coverage for the artifact customers
will use.

Includes a PyPI install in notests_independent_targets because these
seem to be used for Python testing despite -notests.

Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
---
M CMakeLists.txt
M bin/bootstrap_build.sh
M bin/bootstrap_system.sh
M bin/impala-shell.sh
M shell/.gitignore
A shell/CMakeLists.txt
M shell/packaging/make_python_package.sh
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
M tests/shell/util.py
10 files changed, 109 insertions(+), 85 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/86/18586/5
-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................


Patch Set 13: Verified+1

The top of the stack passes tests


-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 13
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 13 Jun 2022 16:41:21 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18586

to look at the new patch set (#2).

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................

IMPALA-11314: Test PyPI package with system python

Sets up a virtualenv with system python to install the impala-shell PyPI
package into. Using system python provides better coverage for Python
versions likely to be used by customers. Runs impala-shell tests using
the PyPI package to provide better coverage for the artifact customers
will use.

Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
---
M CMakeLists.txt
M bin/bootstrap_system.sh
M bin/impala-shell.sh
M shell/.gitignore
A shell/CMakeLists.txt
M shell/packaging/make_python_package.sh
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
M tests/shell/util.py
9 files changed, 105 insertions(+), 83 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/86/18586/2
-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................


Patch Set 11:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10723/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 11
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 07 Jun 2022 19:54:28 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................


Patch Set 12:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18586/12/tests/query_test/test_date_queries.py
File tests/query_test/test_date_queries.py:

http://gerrit.cloudera.org:8080/#/c/18586/12/tests/query_test/test_date_queries.py@27
PS12, Line 27: from tests.shell.util import ImpalaShell, create_impala_shell_executable_dimension
flake8: F401 'tests.shell.util.ImpalaShell' imported but unused



-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 12
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 08 Jun 2022 22:23:26 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................

IMPALA-11314: Test PyPI package with system python

Sets up a virtualenv with system python to install the impala-shell PyPI
package into. Using system python provides better coverage for Python
versions likely to be used by customers. Runs impala-shell tests using
the PyPI package to provide better coverage for the artifact customers
will use.

Includes a PyPI install in notests_independent_targets because these
seem to be used for Python testing despite -notests.

Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Reviewed-on: http://gerrit.cloudera.org:8080/18586
Reviewed-by: Joe McDonnell <jo...@cloudera.com>
Tested-by: Joe McDonnell <jo...@cloudera.com>
---
M CMakeLists.txt
M bin/impala-shell.sh
A bin/impala-virtualenv
A shell/CMakeLists.txt
M tests/custom_cluster/test_client_ssl.py
M tests/query_test/test_date_queries.py
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
M tests/shell/util.py
9 files changed, 140 insertions(+), 110 deletions(-)

Approvals:
  Joe McDonnell: Looks good to me, approved; Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 14
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18586/5/bin/bootstrap_system.sh
File bin/bootstrap_system.sh:

http://gerrit.cloudera.org:8080/#/c/18586/5/bin/bootstrap_system.sh@282
PS5, Line 282: redhat7 sudo yum install -y python-devel python-setuptools python-argparse \
             :         python-virtualenv
> Thanks for the ideas. I'm working on getting CentOS 7 and RedHat (UBI) 8 Do
Using OS virtualenv is more realistic for users, and the requirements.txt for impala-shell specifies a new setuptools. So, we might not hit that issue. I'm ok with using OS virtualenv, as long as it isn't too painful.



-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 03 Jun 2022 21:37:44 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10691/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 03 Jun 2022 00:15:08 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18586/8/shell/CMakeLists.txt
File shell/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/18586/8/shell/CMakeLists.txt@35
PS8, Line 35: add_custom_command(OUTPUT "${PYTHON2_ENV}"
            :   COMMAND impala-virtualenv --python python2 "${PYTHON2_ENV}"
            : )
I think we would want this to depend on impala_python, which is boostrapping impala-python's virtualenv. We don't want both of these running simultaneously.

https://github.com/apache/impala/blob/master/CMakeLists.txt#L473



-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 8
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 06 Jun 2022 18:46:26 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................


Patch Set 9:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18586/9/shell/CMakeLists.txt
File shell/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/18586/9/shell/CMakeLists.txt@29
PS9, Line 29:   COMMAND env BUILD_VERSION=install-test OFFICIAL=true "DIST_DIR=${CMAKE_SOURCE_DIR}/shell/build"
Putting this in shell/build ends up cleaning shell/build without NO_CLEAN_DIST. Can add NO_CLEAN_DIST, or put it in its own subdirectory. I opted to put it in build since it's part of the build process.



-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 9
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 06 Jun 2022 22:11:40 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/18586/2/bin/bootstrap_system.sh
File bin/bootstrap_system.sh:

http://gerrit.cloudera.org:8080/#/c/18586/2/bin/bootstrap_system.sh@281
PS2, Line 281: redhat6 sudo yum install -y python-devel python-setuptools python-argparse python-virtualenv
line too long (92 > 90)


http://gerrit.cloudera.org:8080/#/c/18586/2/bin/bootstrap_system.sh@282
PS2, Line 282: redhat7 sudo yum install -y python-devel python-setuptools python-argparse python-virtualenv
line too long (92 > 90)


http://gerrit.cloudera.org:8080/#/c/18586/2/tests/shell/util.py
File tests/shell/util.py:

http://gerrit.cloudera.org:8080/#/c/18586/2/tests/shell/util.py@334
PS2, Line 334: def create_impala_shell_executable_dimension():
flake8: E302 expected 2 blank lines, found 1



-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 02 Jun 2022 23:55:57 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................


Patch Set 6:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10702/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 03 Jun 2022 17:55:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................


Patch Set 10:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/18586/10/shell/CMakeLists.txt
File shell/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/18586/10/shell/CMakeLists.txt@42
PS10, Line 42:     "${CMAKE_SOURCE_DIR}/shell/build/dist/impala_shell-install-test.tar.gz"
> When I do this, I also have to add a separate custom target to synchronize 
Ok, I don't have very strong feelings about the exact CMake invocations. (And I'm ok if this requires an extra target.)

The thing I do care about is that a developer looking at this file is able to understand that the shell_pypi_test_package produces the file needed by shell_python2_install. Another way to approach this is to add a comment by shell_pypi_test_package that says it generates "${CMAKE_SOURCE_DIR}/shell/build/dist/impala_shell-install-test.tar.gz"


http://gerrit.cloudera.org:8080/#/c/18586/10/tests/shell/util.py
File tests/shell/util.py:

http://gerrit.cloudera.org:8080/#/c/18586/10/tests/shell/util.py@352
PS10, Line 352:   return ImpalaTestDimension('impala_shell_executable',
              :     impala_shell_executable,
              :     os.path.join(IMPALA_HOME, 'shell/build/py2/bin/impala-shell')
One thought came to me: IMPALA_HOME can be in a directory that contains the Jenkins job number/id, so this path could be different for different runs of a Jenkins job (and would be different for different developers). That could mess with automatic analysis of the test results / JUnitXML, so I think we're better off making it consistent across runs.

One way to do that is to avoid including IMPALA_HOME and then add it back in the code that makes use of impala_shell_executable. 

Another way is to have impala_shell_executable be a name of the configuration (e.g. "py2venv", "py3venv", "tarball") and have a lookup table to generate the path.



-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 10
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 07 Jun 2022 18:13:39 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................


Patch Set 9:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10714/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 9
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 06 Jun 2022 20:31:02 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................


Patch Set 10:

(3 comments)

This is looking good to me.

http://gerrit.cloudera.org:8080/#/c/18586/10/shell/CMakeLists.txt
File shell/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/18586/10/shell/CMakeLists.txt@34
PS10, Line 34: set(PYTHON2_ENV "${CMAKE_SOURCE_DIR}/shell/build/py2")
Nit: It might be nice for the name to show it is a virtualenv. Maybe "py2_venv"?


http://gerrit.cloudera.org:8080/#/c/18586/10/shell/CMakeLists.txt@42
PS10, Line 42:     "${CMAKE_SOURCE_DIR}/shell/build/dist/impala_shell-install-test.tar.gz"
Nit: If we wanted to, we could make this a variable and have the "shell_pypi_test_package" be an add_custom_command with this as the OUTPUT.


http://gerrit.cloudera.org:8080/#/c/18586/9/tests/shell/util.py
File tests/shell/util.py:

http://gerrit.cloudera.org:8080/#/c/18586/9/tests/shell/util.py@62
PS9, Line 62: def get_python_version_for_shell_env(impala_shell_executable):
            :   """
            :   Return the version of python belonging to the tested impala_shell_executable.
            : 
            :   We need this because some tests behave differently based on the version of
            :   python being used to execute the impala-shell. However, since the test
            :   framework itself is still being run with python2.7.x, sys.version_info
            :   alone can't help us to determine the python version for the environment of
            :   the shell executable. Instead, we have to invoke the shell, and then parse
            :   the python version from the output. This information is present even in the
            :   case of a fatal shell exception, e.g., not being unable to establish a
            :   connection to an impalad.
            :   """
            :   version_check = Popen([impala_shell_executable, '-q', 'version()'],
            :                         stdout=PIPE, stderr=PIPE, env=build_shell_env())
            :   stdout, stderr = version_check.communicate()
            : 
            :   # e.g. Starting Impala with Kerberos authentication using Python 3.7.6
            :   start_msg_line = stderr.split('\n')[0]
            :   py_version = start_msg_line.split()[-1]   # e.g. 3.7.6
            :   try:
            :     major_version, minor_version, micro_version = py_version.split('.')
            :     ret_val = int(major_version)
            :   except (ValueError, UnboundLocalError) as e:
            :     LOG.error(stderr)
            :     sys.exit("Could not determine python version in shell env: {}".format(str(e)))
            : 
            :   return ret_val
I think SHELL_IS_PYTHON_2 is the only user of this. So, if we no longer need SHELL_IS_PYTHON_2, then we could remove this.

As far as SHELL_IS_PYTHON_2, unless there are very important behavior differences, I'm ok with it being removed.



-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 10
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 07 Jun 2022 00:12:57 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................


Patch Set 7:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10704/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 7
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 03 Jun 2022 21:55:01 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Joe McDonnell, Wenzhe Zhou, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18586

to look at the new patch set (#7).

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................

IMPALA-11314: Test PyPI package with system python

Sets up a virtualenv with system python to install the impala-shell PyPI
package into. Using system python provides better coverage for Python
versions likely to be used by customers. Runs impala-shell tests using
the PyPI package to provide better coverage for the artifact customers
will use.

Includes a PyPI install in notests_independent_targets because these
seem to be used for Python testing despite -notests.

Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
---
M CMakeLists.txt
M bin/impala-shell.sh
A bin/impala-virtualenv
A shell/CMakeLists.txt
M shell/packaging/make_python_package.sh
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
M tests/shell/util.py
8 files changed, 121 insertions(+), 81 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/86/18586/7
-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 7
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10693/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 03 Jun 2022 00:19:07 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................


Patch Set 8:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/18586/5/bin/bootstrap_system.sh
File bin/bootstrap_system.sh:

http://gerrit.cloudera.org:8080/#/c/18586/5/bin/bootstrap_system.sh@282
PS5, Line 282: # Install Python 2.x explicitly for CentOS 8
             : function setup_python2() 
> Using OS virtualenv is more realistic for users, and the requirements.txt f
I don't think the specific version of virtualenv is likely to cause issues. I opted to use the one we already set up to avoid downloading more packages.


http://gerrit.cloudera.org:8080/#/c/18586/8/bin/impala-virtualenv
File bin/impala-virtualenv:

http://gerrit.cloudera.org:8080/#/c/18586/8/bin/impala-virtualenv@20
PS8, Line 20: source "$(dirname "$0")/impala-python-common.sh"
I think I ran into a race-condition with multiple build steps trying to run bootstrap_virtualenv. I need to dig into where we should add an artificial dependency to serialize that.



-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 8
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 06 Jun 2022 18:30:08 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................


Patch Set 10:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/18586/5/bin/bootstrap_system.sh
File bin/bootstrap_system.sh:

http://gerrit.cloudera.org:8080/#/c/18586/5/bin/bootstrap_system.sh@282
PS5, Line 282: # Install Python 2.x explicitly for CentOS 8
             : function setup_python2() 
> I don't think the specific version of virtualenv is likely to cause issues.
Done


http://gerrit.cloudera.org:8080/#/c/18586/5/bin/bootstrap_system.sh@284
PS5, Line 284:   if command -v python && [[ $(python --versio
> I was using virtualenv to generate python2 and python3 environments. venv c
Done


http://gerrit.cloudera.org:8080/#/c/18586/8/bin/impala-virtualenv
File bin/impala-virtualenv:

http://gerrit.cloudera.org:8080/#/c/18586/8/bin/impala-virtualenv@20
PS8, Line 20: source "$(dirname "$0")/impala-python-common.sh"
> I think I ran into a race-condition with multiple build steps trying to run
Done


http://gerrit.cloudera.org:8080/#/c/18586/8/shell/CMakeLists.txt
File shell/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/18586/8/shell/CMakeLists.txt@35
PS8, Line 35: 
            : add_custom_command(OUTPUT "${PYTHON2_ENV}" DEPENDS impala_python
            :  
> I think we would want this to depend on impala_python, which is boostrappin
Done


http://gerrit.cloudera.org:8080/#/c/18586/9/shell/CMakeLists.txt
File shell/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/18586/9/shell/CMakeLists.txt@29
PS9, Line 29:   COMMAND env BUILD_VERSION=install-test OFFICIAL=true
> Putting this in shell/build ends up cleaning shell/build without NO_CLEAN_D
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 10
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 06 Jun 2022 22:38:57 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] Python3 testing

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: Python3 testing
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10689/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 02 Jun 2022 23:05:51 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18586

to look at the new patch set (#3).

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................

IMPALA-11314: Test PyPI package with system python

Sets up a virtualenv with system python to install the impala-shell PyPI
package into. Using system python provides better coverage for Python
versions likely to be used by customers. Runs impala-shell tests using
the PyPI package to provide better coverage for the artifact customers
will use.

Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
---
M CMakeLists.txt
M bin/bootstrap_system.sh
M bin/impala-shell.sh
M shell/.gitignore
A shell/CMakeLists.txt
M shell/packaging/make_python_package.sh
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
M tests/shell/util.py
9 files changed, 108 insertions(+), 83 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/86/18586/3
-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Joe McDonnell, Wenzhe Zhou, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18586

to look at the new patch set (#13).

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................

IMPALA-11314: Test PyPI package with system python

Sets up a virtualenv with system python to install the impala-shell PyPI
package into. Using system python provides better coverage for Python
versions likely to be used by customers. Runs impala-shell tests using
the PyPI package to provide better coverage for the artifact customers
will use.

Includes a PyPI install in notests_independent_targets because these
seem to be used for Python testing despite -notests.

Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
---
M CMakeLists.txt
M bin/impala-shell.sh
A bin/impala-virtualenv
A shell/CMakeLists.txt
M tests/custom_cluster/test_client_ssl.py
M tests/query_test/test_date_queries.py
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
M tests/shell/util.py
9 files changed, 140 insertions(+), 110 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/86/18586/13
-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 13
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................


Patch Set 10:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10716/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 10
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 06 Jun 2022 22:36:08 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Joe McDonnell, Wenzhe Zhou, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18586

to look at the new patch set (#8).

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................

IMPALA-11314: Test PyPI package with system python

Sets up a virtualenv with system python to install the impala-shell PyPI
package into. Using system python provides better coverage for Python
versions likely to be used by customers. Runs impala-shell tests using
the PyPI package to provide better coverage for the artifact customers
will use.

Includes a PyPI install in notests_independent_targets because these
seem to be used for Python testing despite -notests.

Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
---
M CMakeLists.txt
M bin/impala-shell.sh
A bin/impala-virtualenv
M shell/.gitignore
A shell/CMakeLists.txt
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
M tests/shell/util.py
8 files changed, 121 insertions(+), 78 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/86/18586/8
-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 8
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] Python3 testing

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has restored this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: Python3 testing
......................................................................


Restored
-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: restore
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18586

to look at the new patch set (#4).

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................

IMPALA-11314: Test PyPI package with system python

Sets up a virtualenv with system python to install the impala-shell PyPI
package into. Using system python provides better coverage for Python
versions likely to be used by customers. Runs impala-shell tests using
the PyPI package to provide better coverage for the artifact customers
will use.

Includes a PyPI install in notests_independent_targets because these
seem to be used for Python testing despite -notests.

Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
---
M CMakeLists.txt
M bin/bootstrap_system.sh
M bin/impala-shell.sh
M shell/.gitignore
A shell/CMakeLists.txt
M shell/packaging/make_python_package.sh
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
M tests/shell/util.py
9 files changed, 108 insertions(+), 84 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/86/18586/4
-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Joe McDonnell, Wenzhe Zhou, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18586

to look at the new patch set (#12).

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................

IMPALA-11314: Test PyPI package with system python

Sets up a virtualenv with system python to install the impala-shell PyPI
package into. Using system python provides better coverage for Python
versions likely to be used by customers. Runs impala-shell tests using
the PyPI package to provide better coverage for the artifact customers
will use.

Includes a PyPI install in notests_independent_targets because these
seem to be used for Python testing despite -notests.

Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
---
M CMakeLists.txt
M bin/impala-shell.sh
A bin/impala-virtualenv
A shell/CMakeLists.txt
M tests/custom_cluster/test_client_ssl.py
M tests/query_test/test_date_queries.py
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
M tests/shell/util.py
9 files changed, 140 insertions(+), 110 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/86/18586/12
-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 12
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................


Patch Set 10:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/18586/10/shell/.gitignore
File shell/.gitignore:

http://gerrit.cloudera.org:8080/#/c/18586/10/shell/.gitignore@2
PS10, Line 2: /dist/
> The dist directory is moved to build/dist. Do we still need this line?
It's no longer moved to build/dist for direct builds with shell_pypi_package. The test package is going to a different directory in build/.


http://gerrit.cloudera.org:8080/#/c/18586/10/shell/CMakeLists.txt
File shell/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/18586/10/shell/CMakeLists.txt@34
PS10, Line 34: set(PYTHON2_ENV "${CMAKE_SOURCE_DIR}/shell/build/py2")
> Nit: It might be nice for the name to show it is a virtualenv. Maybe "py2_v
Ack


http://gerrit.cloudera.org:8080/#/c/18586/10/shell/CMakeLists.txt@42
PS10, Line 42:     "${CMAKE_SOURCE_DIR}/shell/build/dist/impala_shell-install-test.tar.gz"
> Nit: If we wanted to, we could make this a variable and have the "shell_pyp
I can switch to a custom command for clarity. It'll still rebuild every time because it depends on a custom target.


http://gerrit.cloudera.org:8080/#/c/18586/9/tests/shell/util.py
File tests/shell/util.py:

http://gerrit.cloudera.org:8080/#/c/18586/9/tests/shell/util.py@62
PS9, Line 62: def get_python_version_for_shell_env(impala_shell_executable):
            :   """
            :   Return the version of python belonging to the tested impala_shell_executable.
            : 
            :   We need this because some tests behave differently based on the version of
            :   python being used to execute the impala-shell. However, since the test
            :   framework itself is still being run with python2.7.x, sys.version_info
            :   alone can't help us to determine the python version for the environment of
            :   the shell executable. Instead, we have to invoke the shell, and then parse
            :   the python version from the output. This information is present even in the
            :   case of a fatal shell exception, e.g., not being unable to establish a
            :   connection to an impalad.
            :   """
            :   version_check = Popen([impala_shell_executable, '-q', 'version()'],
            :                         stdout=PIPE, stderr=PIPE, env=build_shell_env())
            :   stdout, stderr = version_check.communicate()
            : 
            :   # e.g. Starting Impala with Kerberos authentication using Python 3.7.6
            :   start_msg_line = stderr.split('\n')[0]
            :   py_version = start_msg_line.split()[-1]   # e.g. 3.7.6
            :   try:
            :     major_version, minor_version, micro_version = py_version.split('.')
            :     ret_val = int(major_version)
            :   except (ValueError, UnboundLocalError) as e:
            :     LOG.error(stderr)
            :     sys.exit("Could not determine python version in shell env: {}".format(str(e)))
            : 
            :   return ret_val
> I think SHELL_IS_PYTHON_2 is the only user of this. So, if we no longer nee
Ack



-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 10
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 07 Jun 2022 16:37:32 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................


Patch Set 11: Code-Review+1


-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 11
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 08 Jun 2022 00:55:12 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................


Patch Set 5:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10699/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 03 Jun 2022 17:19:00 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................


Patch Set 13: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 13
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 13 Jun 2022 16:33:31 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] Python3 testing

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: Python3 testing
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18586/1/shell/make_shell_tarball.sh
File shell/make_shell_tarball.sh:

http://gerrit.cloudera.org:8080/#/c/18586/1/shell/make_shell_tarball.sh@124
PS1, Line 124:     ${IMPALA_PYTHON_EXECUTABLE:-python} -c "import setuptools; exec(open('setup.py').read())" -q bdist_egg
line too long (106 > 90)



-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 02 Jun 2022 22:46:52 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/18586/5/bin/bootstrap_system.sh
File bin/bootstrap_system.sh:

http://gerrit.cloudera.org:8080/#/c/18586/5/bin/bootstrap_system.sh@282
PS5, Line 282: redhat7 sudo yum install -y python-devel python-setuptools python-argparse \
             :         python-virtualenv
virtualenv from the OS can be very old, and it has caused problems in the past. Centos 7 had issues with thrift_sasl 0.4.3 due to old setuptools. We hacked around it for our checked in copy of thrift_sasl, but we don't have that ability for the pypi install.

One alternative is to use the virtualenv/setuptools that we download for our impala-python virtualenv. The download happens before we invoke CMake, so we can assume the tarballs exist at a particular location. (We already do that for infra/python/bootstrap_virtualenv.py.) http://issues.apache.org/jira/browse/IMPALA-10941 tracks using that for building the shell ext-py and undoing the hack for thrift_sasl.

So, one way I could see that working is that we could have a script that will create a python2 virtualenv with the appropriate virtualenv version and setuptools version (read from infra/python/deps/requirements.txt and infra/python/deps/setuptools-requirements.txt). It takes in a directory and puts the virtualenv there.

Our CMake build for the python2 virtualenv can call that rather than using the OS virtualenv executable.

Our make_shell_tarball.sh can also use this script to fix IMPALA-10941.

Optionally, infra/python/bootstrap_virtualenv.py can use that script rather than its current logic.


http://gerrit.cloudera.org:8080/#/c/18586/5/bin/bootstrap_system.sh@284
PS5, Line 284: redhat8 sudo yum install -y python3-virtualenv
Python 3 has a built-in virtualenv mechanism:
https://docs.python.org/3/library/venv.html
So, if you install python3, we can just use that.



-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 03 Jun 2022 20:47:26 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Joe McDonnell, Wenzhe Zhou, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18586

to look at the new patch set (#10).

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................

IMPALA-11314: Test PyPI package with system python

Sets up a virtualenv with system python to install the impala-shell PyPI
package into. Using system python provides better coverage for Python
versions likely to be used by customers. Runs impala-shell tests using
the PyPI package to provide better coverage for the artifact customers
will use.

Includes a PyPI install in notests_independent_targets because these
seem to be used for Python testing despite -notests.

Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
---
M CMakeLists.txt
M bin/impala-shell.sh
A bin/impala-virtualenv
M shell/.gitignore
A shell/CMakeLists.txt
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
M tests/shell/util.py
8 files changed, 122 insertions(+), 78 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/86/18586/10
-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 10
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................


Patch Set 10:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18586/10/shell/CMakeLists.txt
File shell/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/18586/10/shell/CMakeLists.txt@42
PS10, Line 42:     "${CMAKE_SOURCE_DIR}/shell/build/dist/impala_shell-install-test.tar.gz"
> I can switch to a custom command for clarity. It'll still rebuild every tim
When I do this, I also have to add a separate custom target to synchronize so I only build one instance of the test package. Custom commands seem to be rolled into the test targets that depend on them, and Make doesn't see them as a shared dependency between two targets.



-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 10
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 07 Jun 2022 17:38:58 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Joe McDonnell, Wenzhe Zhou, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18586

to look at the new patch set (#11).

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................

IMPALA-11314: Test PyPI package with system python

Sets up a virtualenv with system python to install the impala-shell PyPI
package into. Using system python provides better coverage for Python
versions likely to be used by customers. Runs impala-shell tests using
the PyPI package to provide better coverage for the artifact customers
will use.

Includes a PyPI install in notests_independent_targets because these
seem to be used for Python testing despite -notests.

Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
---
M CMakeLists.txt
M bin/impala-shell.sh
A bin/impala-virtualenv
A shell/CMakeLists.txt
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
M tests/shell/util.py
7 files changed, 134 insertions(+), 107 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/86/18586/11
-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 11
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................


Patch Set 11: Code-Review+1

This is looking good to me. Unless there are other comments, I'll go to +2.


-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 11
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 07 Jun 2022 23:20:50 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................


Patch Set 4:

Build Failed 

https://jenkins.impala.io/job/gerrit-code-review-checks/10697/ : Initial code review checks failed. See linked job for details on the failure.


-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 03 Jun 2022 16:49:19 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18586

to look at the new patch set (#6).

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................

IMPALA-11314: Test PyPI package with system python

Sets up a virtualenv with system python to install the impala-shell PyPI
package into. Using system python provides better coverage for Python
versions likely to be used by customers. Runs impala-shell tests using
the PyPI package to provide better coverage for the artifact customers
will use.

Includes a PyPI install in notests_independent_targets because these
seem to be used for Python testing despite -notests.

Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
---
M CMakeLists.txt
M bin/bootstrap_build.sh
M bin/bootstrap_system.sh
M bin/impala-shell.sh
A shell/CMakeLists.txt
M shell/packaging/make_python_package.sh
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
M tests/shell/util.py
9 files changed, 108 insertions(+), 85 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/86/18586/6
-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] Python3 testing

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has abandoned this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: Python3 testing
......................................................................


Abandoned

Didn't mean to submit this yet (or as part of fixing tar).
-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................


Patch Set 6:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/18586/5/bin/bootstrap_system.sh
File bin/bootstrap_system.sh:

http://gerrit.cloudera.org:8080/#/c/18586/5/bin/bootstrap_system.sh@282
PS5, Line 282: redhat7 sudo yum install -y python-devel python-setuptools python-argparse \
             :         python-virtualenv
> virtualenv from the OS can be very old, and it has caused problems in the p
Thanks for the ideas. I'm working on getting CentOS 7 and RedHat (UBI) 8 Docker environments to test builds. That'll give me coverage on RHEL 7/8 and Ubuntu 16.04/20.04.


http://gerrit.cloudera.org:8080/#/c/18586/5/bin/bootstrap_system.sh@284
PS5, Line 284: redhat8 sudo yum install -y python3-virtualenv
> Python 3 has a built-in virtualenv mechanism:
I was using virtualenv to generate python2 and python3 environments. venv can't do that. However if I need to use the impala-python tools for CentOS 7, I might as well use those everywhere for python2 and use venv for python3.


http://gerrit.cloudera.org:8080/#/c/18586/5/shell/CMakeLists.txt
File shell/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/18586/5/shell/CMakeLists.txt@39
PS5, Line 39: shell/build/dist/
> Keep consistent with the path in shell/packaging/make_python_package.sh
Yeah, I'd changed it there. Joe had made a comment about putting in build/, but I may have misunderstood.



-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 03 Jun 2022 21:01:45 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................


Patch Set 10:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18586/10/shell/.gitignore
File shell/.gitignore:

http://gerrit.cloudera.org:8080/#/c/18586/10/shell/.gitignore@2
PS10, Line 2: /dist/
The dist directory is moved to build/dist. Do we still need this line?



-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 10
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 07 Jun 2022 00:58:47 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18586/8/shell/.gitignore
File shell/.gitignore:

http://gerrit.cloudera.org:8080/#/c/18586/8/shell/.gitignore@2
PS8, Line 2: /dist/
This is not really part of these patches, just something I run into whenever I build the pypi package.



-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 8
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 06 Jun 2022 16:27:19 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Joe McDonnell, Wenzhe Zhou, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18586

to look at the new patch set (#9).

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................

IMPALA-11314: Test PyPI package with system python

Sets up a virtualenv with system python to install the impala-shell PyPI
package into. Using system python provides better coverage for Python
versions likely to be used by customers. Runs impala-shell tests using
the PyPI package to provide better coverage for the artifact customers
will use.

Includes a PyPI install in notests_independent_targets because these
seem to be used for Python testing despite -notests.

Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
---
M CMakeLists.txt
M bin/impala-shell.sh
A bin/impala-virtualenv
M shell/.gitignore
A shell/CMakeLists.txt
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
M tests/shell/util.py
8 files changed, 121 insertions(+), 78 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/86/18586/9
-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 9
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................


Patch Set 13:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10742/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 13
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 08 Jun 2022 22:56:48 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11314: Test PyPI package with system python

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18586 )

Change subject: IMPALA-11314: Test PyPI package with system python
......................................................................


Patch Set 12:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10738/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/18586
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I384ea6a7dab51945828cca629860400a23fa0c05
Gerrit-Change-Number: 18586
Gerrit-PatchSet: 12
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 08 Jun 2022 22:42:30 +0000
Gerrit-HasComments: No