[Impala-ASF-CR] IMPALA-1144: Fix exception when CTRL+C on running query in Impala-shell
David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/8549 ) Change subject: IMPALA-1144: Fix exception when CTRL+C on running query in Impala-shell .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/8549/2/shell/impala_client.py File shell/impala_client.py: http://gerrit.cloudera.org:8080/#/c/8549/2/shell/impala_client.py@336 PS2, Line 336: break; Python abhors a semi-colon. -- To view, visit http://gerrit.cloudera.org:8080/8549 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6cefaf1dae78baae238289816a7cb9d210fb38e2 Gerrit-Change-Number: 8549 Gerrit-PatchSet: 2 Gerrit-Owner: Gabor Kaszab Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 15 Nov 2017 00:40:35 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6148: Specifying thirdparty deps as URLs
David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/8456 ) Change subject: IMPALA-6148: Specifying thirdparty deps as URLs .. Patch Set 8: > Patch Set 6: > > Well, that's what I get for being clever with bash. I've removed the magical > "unset" for loop and am now unsetting every URL explicitly. "It's a big global bag of strings. What could go wrong?" -- To view, visit http://gerrit.cloudera.org:8080/8456 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4628d86022d4bd8b762313f7056d76416a58b422 Gerrit-Change-Number: 8456 Gerrit-PatchSet: 8 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Zach Amsden Gerrit-Comment-Date: Sat, 11 Nov 2017 00:24:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6148: Specifying thirdparty deps as URLs
David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/8456 ) Change subject: IMPALA-6148: Specifying thirdparty deps as URLs .. Patch Set 5: (1 comment) Thanks for starting that gerrit-verify-dryrun job Joe -- I always forget that. http://gerrit.cloudera.org:8080/#/c/8456/5/bin/impala-config.sh File bin/impala-config.sh: http://gerrit.cloudera.org:8080/#/c/8456/5/bin/impala-config.sh@142 PS5, Line 142: unset $var Looks like this is the cause for the gerrit-verify-dryrun failure at https://jenkins.impala.io/job/all-build-options-ub1604/145/console. That Jenkins job relies on a variable called IMPALA_REPO_URL. -- To view, visit http://gerrit.cloudera.org:8080/8456 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4628d86022d4bd8b762313f7056d76416a58b422 Gerrit-Change-Number: 8456 Gerrit-PatchSet: 5 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Zach Amsden Gerrit-Comment-Date: Thu, 09 Nov 2017 16:31:13 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6148: Specifying thirdparty deps as URLs
David Knupp has posted comments on this change. (
http://gerrit.cloudera.org:8080/8456 )
Change subject: IMPALA-6148: Specifying thirdparty deps as URLs
..
Patch Set 4: Code-Review+2
(1 comment)
> Patch Set 4:
>
> (1 comment)
http://gerrit.cloudera.org:8080/#/c/8456/2/bin/bootstrap_toolchain.py
File bin/bootstrap_toolchain.py:
http://gerrit.cloudera.org:8080/#/c/8456/2/bin/bootstrap_toolchain.py@112
PS2, Line 112: if re.search(k, release):
> The 57 times is why we're adding the cache. It was spamming the log, mostly
Got it. Thanks for pointing that line out. It does seem to me that baking in
platform as an attribute of Package fits the intention of the class -- perhaps
with a default value, but override-able such that L240 might look like:
Package("kudu", kudu_version, "centos7")
But I get not wanting to get into excessive refactoring of existing code, and
for now not spamming the log is an improvement, so I think what you have here
is fine.
--
To view, visit http://gerrit.cloudera.org:8080/8456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4628d86022d4bd8b762313f7056d76416a58b422
Gerrit-Change-Number: 8456
Gerrit-PatchSet: 4
Gerrit-Owner: Philip Zeyliger
Gerrit-Reviewer: David Knupp
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Zach Amsden
Gerrit-Comment-Date: Wed, 08 Nov 2017 21:58:18 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6148: Specifying thirdparty deps as URLs
David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/8456 ) Change subject: IMPALA-6148: Specifying thirdparty deps as URLs .. Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/8456/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8456/4//COMMIT_MSG@10 PS4, Line 10: , Nit: this comma is probably not necessary; it actually kind of makes the sentence less clear. http://gerrit.cloudera.org:8080/#/c/8456/2/bin/bootstrap_toolchain.py File bin/bootstrap_toolchain.py: http://gerrit.cloudera.org:8080/#/c/8456/2/bin/bootstrap_toolchain.py@112 PS2, Line 112: if re.search(k, release): > You can't trivially cache v because it depends on the argument 'release'. This is probably an obtuse question (you can count on me for those): for any given invocation of this script, won't release always be the same thing? E.g., if I'm bootstrapping the toolchain on my dev machine, the release will only ever resolve to ubuntu14.04, no matter how many times "lsb_release -irs" gets called (57 times, it turns out -- fewer if parts of the toolchain are already there.) -- To view, visit http://gerrit.cloudera.org:8080/8456 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4628d86022d4bd8b762313f7056d76416a58b422 Gerrit-Change-Number: 8456 Gerrit-PatchSet: 4 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Zach Amsden Gerrit-Comment-Date: Wed, 08 Nov 2017 20:54:49 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6148: Specifying thirdparty deps as URLs
David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/8456 ) Change subject: IMPALA-6148: Specifying thirdparty deps as URLs .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/8456/2/bin/bootstrap_toolchain.py File bin/bootstrap_toolchain.py: http://gerrit.cloudera.org:8080/#/c/8456/2/bin/bootstrap_toolchain.py@112 PS2, Line 112: return v > Why not cache v instead? Also, would it be appropriate to cache the value as an attribute of Package() instead? -- To view, visit http://gerrit.cloudera.org:8080/8456 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4628d86022d4bd8b762313f7056d76416a58b422 Gerrit-Change-Number: 8456 Gerrit-PatchSet: 3 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Zach Amsden Gerrit-Comment-Date: Tue, 07 Nov 2017 17:29:57 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6148: Specifying thirdparty deps as URLs
David Knupp has posted comments on this change. (
http://gerrit.cloudera.org:8080/8456 )
Change subject: IMPALA-6148: Specifying thirdparty deps as URLs
..
Patch Set 3:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/8456/2/bin/bootstrap_toolchain.py
File bin/bootstrap_toolchain.py:
http://gerrit.cloudera.org:8080/#/c/8456/2/bin/bootstrap_toolchain.py@36
PS2, Line 36: import logging
Not a blocker, but would it make sense to find/replace print -> logging
throughout?
http://gerrit.cloudera.org:8080/#/c/8456/2/bin/bootstrap_toolchain.py@82
PS2, Line 82: raise Exception("Could not find version for {0} in
environment var {1}".format(
Nit: Probably a bit pedantic, but catching a specific exception only to raise a
generic one looks weird. Perhaps...
msg = "Could not find version for {0} in environment var {1}".format(name,
version_env_var)
raise KeyError(msg)
...or something similar.
http://gerrit.cloudera.org:8080/#/c/8456/2/bin/bootstrap_toolchain.py@373
PS2, Line 373: pkg_directory = package_directory(cdh_components_home,
component.name,
Nit: trailing white space
http://gerrit.cloudera.org:8080/#/c/8456/2/bin/bootstrap_toolchain.py@399
PS2, Line 399: logging.getLogger("sh").setLevel(logging.WARNING)
Nice.
--
To view, visit http://gerrit.cloudera.org:8080/8456
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4628d86022d4bd8b762313f7056d76416a58b422
Gerrit-Change-Number: 8456
Gerrit-PatchSet: 3
Gerrit-Owner: Philip Zeyliger
Gerrit-Reviewer: David Knupp
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Zach Amsden
Gerrit-Comment-Date: Tue, 07 Nov 2017 01:59:37 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6070: Parallel compute table stats.py
David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/8354 ) Change subject: IMPALA-6070: Parallel compute_table_stats.py .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/8354 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifb080f2552b9dbe304ecadd6e52429214094237d Gerrit-Change-Number: 8354 Gerrit-PatchSet: 4 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Mon, 23 Oct 2017 21:09:10 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6070: Parallel compute table stats.py
David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/8354 ) Change subject: IMPALA-6070: Parallel compute_table_stats.py .. Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/8354/2/tests/util/compute_table_stats.py File tests/util/compute_table_stats.py: http://gerrit.cloudera.org:8080/#/c/8354/2/tests/util/compute_table_stats.py@48 PS2, Line 48: ror=False, par > It's a closure. Doing it inline in the main block lets the closure access o Thanks for the clarification. http://gerrit.cloudera.org:8080/#/c/8354/2/tests/util/compute_table_stats.py@71 PS2, Line 71: end(pool.apply_async(compute_stats_table > Let's say all_tables = ["a", "b", "c"] and selected_tables = ["b", "c", "d" Thanks for the clarification. I actually missed that you do the same thing for dbs on L67. I wasn't considering the fact that selected_tables might contain a table that's not in all_tables, or that selected_dbs might contain a db that's not in all_dbs. So one last probably naive question. Does screening out values that don't exist in all_* represent a possible silent failure -- say, if someone invokes the script with --db_names followed by a misspelled value? -- To view, visit http://gerrit.cloudera.org:8080/8354 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifb080f2552b9dbe304ecadd6e52429214094237d Gerrit-Change-Number: 8354 Gerrit-PatchSet: 4 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Mon, 23 Oct 2017 19:14:29 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6070: Parallel compute table stats.py
David Knupp has posted comments on this change. (
http://gerrit.cloudera.org:8080/8354 )
Change subject: IMPALA-6070: Parallel compute_table_stats.py
..
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/8354/2/tests/util/compute_table_stats.py
File tests/util/compute_table_stats.py:
http://gerrit.cloudera.org:8080/#/c/8354/2/tests/util/compute_table_stats.py@44
PS2, Line 44: if not continue_on_error: raise e
Two nits: even though we tend to do this a lot, PEP 8 frowns on inline if
statements.
Also, I believe that if you're not handling an exception, but rather just
logging something and re-raising, then a bare raise statement is preferred. I'm
not even sure you need to reference the base Exception class in this case,
since you're catching all exceptions. E.g.
try:
fail()
except:
logging.exception("oops!")
raise
http://gerrit.cloudera.org:8080/#/c/8354/2/tests/util/compute_table_stats.py@48
PS2, Line 48:
I'm curious -- why is it necessary to define the client factory inside of the
if __name__ == "__main__" block, and then pass a reference to it here? Wouldn't
defining the method in the top-level module namespace be just as effective? It
might be obvious, but I'm missing it.
http://gerrit.cloudera.org:8080/#/c/8354/2/tests/util/compute_table_stats.py@71
PS2, Line 71: _factory, db, table, continue_on_error,)
It seems to me like you don't need the intersection. You already have either
set(all_tables) or set(table_names). Why not just:
for table in selected_tables:
?
--
To view, visit http://gerrit.cloudera.org:8080/8354
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb080f2552b9dbe304ecadd6e52429214094237d
Gerrit-Change-Number: 8354
Gerrit-PatchSet: 3
Gerrit-Owner: Philip Zeyliger
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: David Knupp
Gerrit-Reviewer: Michael Brown
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Comment-Date: Mon, 23 Oct 2017 18:32:31 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6070: Parallel compute table stats.py
David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/8354 ) Change subject: IMPALA-6070: Parallel compute_table_stats.py .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/8354/1/tests/util/compute_table_stats.py File tests/util/compute_table_stats.py: http://gerrit.cloudera.org:8080/#/c/8354/1/tests/util/compute_table_stats.py@37 PS1, Line 37: statement = "compute stats %s" % (db_table,) > They do. At first I thought I didn't care, but you're right, and I've fixed Thank you! I wish we could get rid of the spurious print statements in our code. If we ever have to be python3 compatible, those will cause us a lot of grief. -- To view, visit http://gerrit.cloudera.org:8080/8354 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifb080f2552b9dbe304ecadd6e52429214094237d Gerrit-Change-Number: 8354 Gerrit-PatchSet: 2 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Mon, 23 Oct 2017 17:52:48 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6070: Parallel compute table stats.py
David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/8354 ) Change subject: IMPALA-6070: Parallel compute_table_stats.py .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/8354/2/tests/util/compute_table_stats.py File tests/util/compute_table_stats.py: http://gerrit.cloudera.org:8080/#/c/8354/2/tests/util/compute_table_stats.py@116 PS2, Line 116: def client_factory(): Would it be possible to decorate client_factory as a @contextmanager, something like: from contextlib import contextmanager @contextmanager def client_factory(): impala_client = ImpalaBeeswaxClient(options.impalad, use_kerberos=options.use_kerberos, use_ssl=options.use_ssl) impala_client.connect() yield impala_client impala_client.close_connection() ...such that usage becomes: with client_factory() as impala_client: # etc... Then you could then do away with the try/finally for closing the connection. -- To view, visit http://gerrit.cloudera.org:8080/8354 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifb080f2552b9dbe304ecadd6e52429214094237d Gerrit-Change-Number: 8354 Gerrit-PatchSet: 2 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Mon, 23 Oct 2017 17:50:55 + Gerrit-HasComments: Yes
[Impala-ASF-CR](asf-site) Remove old leftover, unmaintained parts of website.
David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/8234 ) Change subject: Remove old leftover, unmaintained parts of website. .. Patch Set 1: Code-Review+2 Seems reasonable to me. -- To view, visit http://gerrit.cloudera.org:8080/8234 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: asf-site Gerrit-MessageType: comment Gerrit-Change-Id: Ia437d37c40d6cc2c8491cc9695f7ce27501b6797 Gerrit-Change-Number: 8234 Gerrit-PatchSet: 1 Gerrit-Owner: Jim Apple Gerrit-Reviewer: David Knupp Gerrit-Comment-Date: Mon, 09 Oct 2017 21:40:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5376: Implement all TPCDS test cases or alternates for Impala.
David Knupp has posted comments on this change. (
http://gerrit.cloudera.org:8080/8102 )
Change subject: IMPALA-5376: Implement all TPCDS test cases or alternates for
Impala.
..
Patch Set 6:
Tim, can you provide more context for the observation that "run-tests.py exits
non-zero when an expected-fail test is executed?" We commonly have XFAIL'ed
tests that do not cause jobs to fail, e.g. from
https://jenkins.impala.io/job/ubuntu-16.04-from-scratch/320/consoleFull
03:28:03 XFAIL
failure/test_failpoints.py::TestFailpoints::()::test_failpoints[table_format:
hbase/none | exec_option: {'batch_size': 0, 'num_nodes': 0,
'disable_codegen_rows_threshold': 0, 'disable_codegen': False,
'abort_on_error': 1, 'exec_single_node_rows_threshold': 0} | mt_dop: 4 |
location: PREPARE | action: FAIL | query: select 1 from alltypessmall a join
alltypessmall b on a.id = b.id]
03:28:03 reason: MT_DOP not supported.
03:28:03 XFAIL
query_test/test_insert_behaviour.py::TestInsertBehaviour::()::test_insert_inherit_acls
03:28:03 reason: [NOTRUN] Fails intermittently on test clusters
03:28:03 === 1774 tests deselected by "-m 'execute_serially'"
===
03:28:03 = 178 passed, 8 skipped, 1774 deselected, 4 xfailed in 1966.41
seconds =
--
To view, visit http://gerrit.cloudera.org:8080/8102
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6e284888600a7a69d1f23fcb7dac21cbb13b7d66
Gerrit-Change-Number: 8102
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Wood
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: David Knupp
Gerrit-Reviewer: Matthew Mulder
Gerrit-Reviewer: Michael Brown
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Mostafa Mokhtar
Gerrit-Reviewer: Tim Wood
Gerrit-Comment-Date: Mon, 25 Sep 2017 18:08:36 +
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5376: Implement all TPCDS test cases or alternates for Impala.
David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/8102 ) Change subject: IMPALA-5376: Implement all TPCDS test cases or alternates for Impala. .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/8102/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8102/5//COMMIT_MSG@12 PS5, Line 12: This commit continues previous work on IMPALA-5376 under the apache/incubator-impala repo : on github.com, and commit 6877 at gerrit.cloudera.org:8080. > Done Hope this isn't just me being a pest, but when I look at IMPALA-5376 on the Apache JIRA server, there's simply nothing there indicating that an initial patch (adding all of the missing tables) was earlier committed by Michael Ho. I prefer to have it explicitly stated this is a follow-on to that earlier patch. The last suggestion in this regard was to reference the first patch by its title, "IMPALA-5376: Loads all TPC-DS tables," rather than using a transient gerrit or github identifier. -- To view, visit http://gerrit.cloudera.org:8080/8102 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6e284888600a7a69d1f23fcb7dac21cbb13b7d66 Gerrit-Change-Number: 8102 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Wood Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Matthew Mulder Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Mostafa Mokhtar Gerrit-Reviewer: Tim Wood Gerrit-Comment-Date: Fri, 22 Sep 2017 21:58:21 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5376: Implement all TPCDS test cases or alternates for Impala.
David Knupp has posted comments on this change. Change subject: IMPALA-5376: Implement all TPCDS test cases or alternates for Impala. .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/8102/1//COMMIT_MSG Commit Message: Line 11: Data set constructed in mini-cluster using incubator-impala/bin/buildall.sh -testdata > I think this sentence is awkward. "incubator-impala" happens to be what the mikeb pointed out to me that a searchable string, like the previous commits title -- "IMPALA-5376: Loads all TPC-DS tables" -- is preferred. URL's can change. -- To view, visit http://gerrit.cloudera.org:8080/8102 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6e284888600a7a69d1f23fcb7dac21cbb13b7d66 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Wood Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Mostafa Mokhtar Gerrit-Reviewer: Tim Wood Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5376: Implement all TPCDS test cases or alternates for Impala.
David Knupp has posted comments on this change.
Change subject: IMPALA-5376: Implement all TPCDS test cases or alternates for
Impala.
..
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8102/1//COMMIT_MSG
Commit Message:
Line 11: Data set constructed in mini-cluster using
incubator-impala/bin/buildall.sh -testdata
I think this sentence is awkward. "incubator-impala" happens to be what the
directory is called on your system. I suspect that most people have renamed
their local directories to "Impala." A better choice would be
${IMPALA_HOME}/bin/etc...
I also think it might be helpful to reference this patch, although I'm not sure
which link preferable. Yo might want to solicit opinions on that.
https://github.com/apache/incubator-impala/commit/f1558957
http://gerrit.cloudera.org:8080/6877
--
To view, visit http://gerrit.cloudera.org:8080/8102
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6e284888600a7a69d1f23fcb7dac21cbb13b7d66
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Wood
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: David Knupp
Gerrit-Reviewer: Michael Brown
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Mostafa Mokhtar
Gerrit-Reviewer: Tim Wood
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5941: Fix Metastore schema creation in create-test-configuration.sh
David Knupp has posted comments on this change. Change subject: IMPALA-5941: Fix Metastore schema creation in create-test-configuration.sh .. Patch Set 4: This was submitted by hand. I accidentally had clicked "dry_run" in jenkins.impala.io. -- To view, visit http://gerrit.cloudera.org:8080/8081 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic312df4597c7d211d4ecd551d572f751aea0cd24 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5941: Fix Metastore schema creation in create-test-configuration.sh
David Knupp has posted comments on this change. Change subject: IMPALA-5941: Fix Metastore schema creation in create-test-configuration.sh .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/8081 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic312df4597c7d211d4ecd551d572f751aea0cd24 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5941: Fix Metastore schema creation in create-test-configuration.sh
David Knupp has submitted this change and it was merged. Change subject: IMPALA-5941: Fix Metastore schema creation in create-test-configuration.sh .. IMPALA-5941: Fix Metastore schema creation in create-test-configuration.sh The Hive Metastore schema script includes other SQL scripts using \i, which expects absolute paths. Since we currently invoke it from outside the schema script directory, it is unable to find those included scripts. The fix is to switch to the Hive Metastore script directory when invoking the schema script. Change-Id: Ic312df4597c7d211d4ecd551d572f751aea0cd24 Reviewed-on: http://gerrit.cloudera.org:8080/8081 Reviewed-by: Tim Armstrong Tested-by: David Knupp --- M bin/create-test-configuration.sh 1 file changed, 5 insertions(+), 2 deletions(-) Approvals: David Knupp: Verified Tim Armstrong: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/8081 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ic312df4597c7d211d4ecd551d572f751aea0cd24 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-5941: Fix Metastore schema creation in create-test-configuration.sh
David Knupp has posted comments on this change. Change subject: IMPALA-5941: Fix Metastore schema creation in create-test-configuration.sh .. Patch Set 3: Started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1234/ -- To view, visit http://gerrit.cloudera.org:8080/8081 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic312df4597c7d211d4ecd551d572f751aea0cd24 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5617: Include full workload name in tpch nested query filenames
David Knupp has posted comments on this change. Change subject: IMPALA-5617: Include full workload name in tpch_nested query filenames .. Patch Set 3: Code-Review+2 In order to get this change in, you'll need a committer to run the Gerritt-Verify-Dryrun job for you. I'll do that now. -- To view, visit http://gerrit.cloudera.org:8080/7891 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie067b201ae20b4f4c61a98be7ac1ec5a3f8febd8 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Wood Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Matthew Mulder Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Tim Wood Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5617: Include full workload name in tpch nested query filenames
David Knupp has posted comments on this change. Change subject: IMPALA-5617: Include full workload name in tpch_nested query filenames .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/7891/3/tests/query_test/test_tpch_nested_queries.py File tests/query_test/test_tpch_nested_queries.py: Line 36: def test_tpch_q1(self, vector): > So, I'm going to offer a comment in the spirit of this being a good opportu Actually, on later reflection, the vector might complicate this. If this seems hard to do, or makes test reporting hard to parse, then feel free to ignore this. -- To view, visit http://gerrit.cloudera.org:8080/7891 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie067b201ae20b4f4c61a98be7ac1ec5a3f8febd8 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Wood Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Matthew Mulder Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Tim Wood Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5617: Include full workload name in tpch nested query filenames
David Knupp has posted comments on this change.
Change subject: IMPALA-5617: Include full workload name in tpch_nested query
filenames
..
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7891/3/tests/query_test/test_tpch_nested_queries.py
File tests/query_test/test_tpch_nested_queries.py:
Line 36: def test_tpch_q1(self, vector):
So, I'm going to offer a comment in the spirit of this being a good opportunity
to learn something about pytest. If we use the pytest parametrize marker, all
of these test functions can be collapsed to a single function, something like
this (untested code):
queries_list = ['q1', 'q2', 'q3', ...]
@pytest.mark.parametrize('query', TestTpchNestedQuery.queries_list)
def test_tpch_nested_query(self, query, vector):
self.run_test_case('{0}-{1}'.format(self.get_workload(), query), vector)
A test run would look something like:
test_params.py::test_tpch_nested_query[q1] PASSED
test_params.py::test_tpch_nested_query[q2] PASSED
test_params.py::test_tpch_nested_query[q3] PASSED
etc...
This would greatly reduce the tedium of any future refactoring.
That said, I'm happy to +2 to this as is if you'd rather not tackle that. Just
let me know in a reply comment.
--
To view, visit http://gerrit.cloudera.org:8080/7891
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie067b201ae20b4f4c61a98be7ac1ec5a3f8febd8
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Wood
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: David Knupp
Gerrit-Reviewer: Matthew Mulder
Gerrit-Reviewer: Michael Brown
Gerrit-Reviewer: Tim Wood
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5617: Rename tpch nested query files per pattern
David Knupp has posted comments on this change. Change subject: IMPALA-5617: Rename tpch_nested query files per pattern .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/7891/2/tests/query_test/test_tpch_nested_queries.py File tests/query_test/test_tpch_nested_queries.py: PS2, Line 37: str get_workload() already returns a string. Wrapping this in str() seems redundant. -- To view, visit http://gerrit.cloudera.org:8080/7891 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie067b201ae20b4f4c61a98be7ac1ec5a3f8febd8 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Wood Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Matthew Mulder Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Tim Wood Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5820: Fix string format() syntax in test scanners fuzz.py
David Knupp has posted comments on this change. Change subject: IMPALA-5820: Fix string format() syntax in test_scanners_fuzz.py .. Patch Set 4: Code-Review+2 Rebase only, carrying +2. -- To view, visit http://gerrit.cloudera.org:8080/7761 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5384aaf83a6a1f3c7643ed9f15de2dba1a5913a5 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: David Knupp Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5820: Fix string format() syntax in test scanners fuzz.py
David Knupp has posted comments on this change. Change subject: IMPALA-5820: Fix string format() syntax in test_scanners_fuzz.py .. Patch Set 4: I was stupid. I specifically tested my patch against the RELEASE build, which didn't need Tim's dcheck fix for IMPALA-5819. However, when this change was tested upstream, it ran against a DEBUG build, and without Tim's change, it failed. -- To view, visit http://gerrit.cloudera.org:8080/7761 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5384aaf83a6a1f3c7643ed9f15de2dba1a5913a5 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: David Knupp Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5820: Fix string format() syntax in test scanners fuzz.py
David Knupp has uploaded a new patch set (#3).
Change subject: IMPALA-5820: Fix string format() syntax in test_scanners_fuzz.py
..
IMPALA-5820: Fix string format() syntax in test_scanners_fuzz.py
Make the call to format() compatible with older versions of python
(< 2.7), which expect an indices in the string being formatted,
e.g. "{0} {1} {2}".format('foo', 'bar', 'baz'). Without the numbers,
format() raises an exception.
Tested by running this test suite using python 2.6.6. Before the
patch, the tests failed. After the patch, they pass.
Change-Id: I5384aaf83a6a1f3c7643ed9f15de2dba1a5913a5
---
M tests/query_test/test_scanners_fuzz.py
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/7761/3
--
To view, visit http://gerrit.cloudera.org:8080/7761
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5384aaf83a6a1f3c7643ed9f15de2dba1a5913a5
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Michael Brown
[Impala-ASF-CR] IMPALA-5820: Fix string format() syntax in test scanners fuzz.py
David Knupp has uploaded a new patch set (#3).
Change subject: IMPALA-5820: Fix string format() syntax in test_scanners_fuzz.py
..
IMPALA-5820: Fix string format() syntax in test_scanners_fuzz.py
Make the call to format() compatible with older versions of python
(< 2.7), which expect an indices in the string being formatted,
e.g. "{0} {1} {2}".format('foo', 'bar', 'baz'). Without the numbers,
format() raises an exception.
Tested by running this test suite using python 2.6.6. Before the
patch, the tests failed. After the patch, they pass.
Change-Id: I5384aaf83a6a1f3c7643ed9f15de2dba1a5913a5
---
M tests/query_test/test_scanners_fuzz.py
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/7761/3
--
To view, visit http://gerrit.cloudera.org:8080/7761
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5384aaf83a6a1f3c7643ed9f15de2dba1a5913a5
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Michael Brown
[Impala-ASF-CR] Fix string format() syntax to in test scanners fuzz.py
David Knupp has uploaded a new change for review.
http://gerrit.cloudera.org:8080/7761
Change subject: Fix string format() syntax to in test_scanners_fuzz.py
..
Fix string format() syntax to in test_scanners_fuzz.py
Make the call to format() compatible with older versions of python
(< 2.7), which expect an indices in the string being formatted,
e.g. "{0} {1} {2}".format('foo', 'bar', 'baz'). Without the numbers,
format() raises an exception.
Tested by running this test suite using python 2.6.6. Before the
patch, the tests failed. After the patch, they pass.
Change-Id: I5384aaf83a6a1f3c7643ed9f15de2dba1a5913a5
---
M tests/query_test/test_scanners_fuzz.py
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/7761/2
--
To view, visit http://gerrit.cloudera.org:8080/7761
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5384aaf83a6a1f3c7643ed9f15de2dba1a5913a5
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp
[Impala-ASF-CR] IMPALA-5009: Clean up test insert parquet.py
David Knupp has posted comments on this change. Change subject: IMPALA-5009: Clean up test_insert_parquet.py .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7518 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia84c78d7ff74cc7fdb3d782060caa5e52d0cd7d2 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars Volker Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5009: Clean up test insert parquet.py
David Knupp has posted comments on this change. Change subject: IMPALA-5009: Clean up test_insert_parquet.py .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/7518/2/tests/query_test/test_insert_parquet.py File tests/query_test/test_insert_parquet.py: PS2, Line 407: tmpdir Looks like you missed a spot. -- To view, visit http://gerrit.cloudera.org:8080/7518 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia84c78d7ff74cc7fdb3d782060caa5e52d0cd7d2 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars Volker Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5009: Clean up test insert parquet.py
David Knupp has posted comments on this change. Change subject: IMPALA-5009: Clean up test_insert_parquet.py .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/7518/1/tests/query_test/test_insert_parquet.py File tests/query_test/test_insert_parquet.py: PS1, Line 351: tmpdir.strpath Adding on to Michael's comment, rather than passing the whole fixture around, you might even consider just passing tmpdir.strpath initially, since that seems to be the only thing you ultimately need to access in this method. -- To view, visit http://gerrit.cloudera.org:8080/7518 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia84c78d7ff74cc7fdb3d782060caa5e52d0cd7d2 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars Volker Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Michael Brown Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3496: stress test: print version info
David Knupp has posted comments on this change. Change subject: IMPALA-3496: stress test: print version info .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7501 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie4b40783ddae6b1bfb2bb4e28c0e3bf97ab944c5 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Brown Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Matthew Mulder Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4868: Disable flaky TestRequestPoolService test
David Knupp has posted comments on this change. Change subject: IMPALA-4868: Disable flaky TestRequestPoolService test .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7466 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I616961457cd48d31d618c8b58f5279b89d3cdcd6 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs Gerrit-Reviewer: David Knupp Gerrit-HasComments: No
[native-toolchain-CR] Prevent leaking AWS credentials to the log.
David Knupp has posted comments on this change. Change subject: Prevent leaking AWS credentials to the log. .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7391 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I43ae8d6f347d0ce3d223bd6e86759bb33f2520e6 Gerrit-PatchSet: 2 Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-Owner: Lars Volker Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Tim Wood Gerrit-HasComments: No
[native-toolchain-CR] Prevent leaking AWS credentials to the log.
David Knupp has posted comments on this change. Change subject: Prevent leaking AWS credentials to the log. .. Patch Set 2: Code-Review+1 Lars asked me to look at this as well, and I feel the same way. Tim Wood is better at bash than me. I'll confer with him, then +2 if things check out. -- To view, visit http://gerrit.cloudera.org:8080/7391 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I43ae8d6f347d0ce3d223bd6e86759bb33f2520e6 Gerrit-PatchSet: 2 Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-Owner: Lars Volker Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Matthew Jacobs Gerrit-HasComments: No
[Impala-ASF-CR] Complete guide to important environment variables for build, test, and mini-cluster operations.
David Knupp has posted comments on this change. Change subject: Complete guide to important environment variables for build, test, and mini-cluster operations. .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/7350/1/docs/Developer-Guide/Build-Environment.txt File docs/Developer-Guide/Build-Environment.txt: Line 1: Environment Variables: > Additional docs will be forthcoming I may be missing something, but there seem to be a few missing settings, e.g., these from testdata/bin/cluster/admin export HDFS_WEBUI_PORT=5070 # changed from 50070 so it is not ephemeral export YARN_WEBUI_PORT=8088 # same as default export KMS_WEBUI_PORT=9600# changed to make room for non-ephemeral HBase ports # (HADOOP-12811) export KUDU_WEBUI_PORT=8051 # same as default -- To view, visit http://gerrit.cloudera.org:8080/7350 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16d34cb4fa0c60c5ad6d9c8764cc0ec21c5cb368 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Zach Amsden Gerrit-HasComments: Yes
[Impala-ASF-CR] Complete guide to important environment variables for build, test, and mini-cluster operations.
David Knupp has posted comments on this change. Change subject: Complete guide to important environment variables for build, test, and mini-cluster operations. .. Patch Set 1: (2 comments) Whether it's a .txt file in the code base, or a wiki entry, how can we make sure that this doc stays in sync with impala-config.sh (and/or any other place where env vars get set)? We'd have multiple sources of truth, wouldn't we? http://gerrit.cloudera.org:8080/#/c/7350/1/bin/impala-config.sh File bin/impala-config.sh: Line 440 Speaking confidently as the only team member with a fine arts education, I'm shocked that I did not know this setting existed. :-) http://gerrit.cloudera.org:8080/#/c/7350/1/docs/Developer-Guide/Build-Environment.txt File docs/Developer-Guide/Build-Environment.txt: Line 1: Environment Variables: How many of these are set in impala-config.sh, and how many are set by other means? -- To view, visit http://gerrit.cloudera.org:8080/7350 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16d34cb4fa0c60c5ad6d9c8764cc0ec21c5cb368 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Zach Amsden Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5281: stress test: introduce stricter pass guidelines
David Knupp has posted comments on this change. Change subject: IMPALA-5281: stress test: introduce stricter pass guidelines .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7282 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9f2174a527193ae01be45b8ed56315c465883346 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Brown Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Matthew Mulder Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5281: stress test: introduce stricter pass guidelines
David Knupp has posted comments on this change. Change subject: IMPALA-5281: stress test: introduce stricter pass guidelines .. Patch Set 1: >From a code perspective, it's straightforward. But Matt may have more stress >test knowledge than I do re: the implications. When he gives it a thumbs up, I >can +2 it. -- To view, visit http://gerrit.cloudera.org:8080/7282 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9f2174a527193ae01be45b8ed56315c465883346 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Brown Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Matthew Mulder Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5517: Allow IMPALA LOGS DIR to be overridden
David Knupp has posted comments on this change. Change subject: IMPALA-5517: Allow IMPALA_LOGS_DIR to be overridden .. Patch Set 3: Code-Review+2 Rebase only. Carrying +2. -- To view, visit http://gerrit.cloudera.org:8080/7197 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4028813bd4f53815139225abd57845bb304ae3e4 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: David Knupp Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Michael Brown Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5517: Allow IMPALA LOGS DIR to be overridden
David Knupp has posted comments on this change. Change subject: IMPALA-5517: Allow IMPALA_LOGS_DIR to be overridden .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/7197/1//COMMIT_MSG Commit Message: PS1, Line 7: IMPALA-5517: Allow default IMPALA_LOGS_DIR to be explicity overridden > Nit: wording. Maybe: Done -- To view, visit http://gerrit.cloudera.org:8080/7197 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4028813bd4f53815139225abd57845bb304ae3e4 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: David Knupp Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Michael Brown Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5517: Allow IMPALA LOGS DIR to be overridden
David Knupp has uploaded a new patch set (#2). Change subject: IMPALA-5517: Allow IMPALA_LOGS_DIR to be overridden .. IMPALA-5517: Allow IMPALA_LOGS_DIR to be overridden Tested by exporting a different IMPALA_LOGS_DIR path, then confirming that it was not changed by sourcing impala-config.sh. Change-Id: I4028813bd4f53815139225abd57845bb304ae3e4 --- M bin/impala-config.sh 1 file changed, 2 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/97/7197/2 -- To view, visit http://gerrit.cloudera.org:8080/7197 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4028813bd4f53815139225abd57845bb304ae3e4 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: David Knupp Gerrit-Reviewer: Michael Brown
[Impala-ASF-CR] IMPALA-5517: Allow default IMPALA LOGS DIR to be explicity overridden
David Knupp has uploaded a new change for review. http://gerrit.cloudera.org:8080/7197 Change subject: IMPALA-5517: Allow default IMPALA_LOGS_DIR to be explicity overridden .. IMPALA-5517: Allow default IMPALA_LOGS_DIR to be explicity overridden Tested by exporting a different IMPALA_LOGS_DIR path, then confirming that it was not changed by sourcing impala-config.sh. Change-Id: I4028813bd4f53815139225abd57845bb304ae3e4 --- M bin/impala-config.sh 1 file changed, 2 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/97/7197/1 -- To view, visit http://gerrit.cloudera.org:8080/7197 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I4028813bd4f53815139225abd57845bb304ae3e4 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: David Knupp
[Impala-ASF-CR] IMPALA-3040 addendum: use specific build type timeout for slow builds
David Knupp has posted comments on this change. Change subject: IMPALA-3040 addendum: use specific_build_type_timeout for slow builds .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7115 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I80f1c8a0e634a3726c53ef7297c5b162dd57a3a2 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Brown Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5223: Add waiting for HBase Zookeeper nodes to retry loop
David Knupp has posted comments on this change. Change subject: IMPALA-5223: Add waiting for HBase Zookeeper nodes to retry loop .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7159 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id8dbdff4ad02cac1322e7d580e0a6971daf6ea28 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars Volker Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: anujphadke Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5263: test infra: support CA bundles with secure clusters
David Knupp has posted comments on this change. Change subject: IMPALA-5263: test infra: support CA bundles with secure clusters .. Patch Set 2: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/7152/2/tests/comparison/cli_options.py File tests/comparison/cli_options.py: Line 205: 'once', "once" Nice. I've never even heard of this library before. -- To view, visit http://gerrit.cloudera.org:8080/7152 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifb9e466e4b7cde704cdc4cf98159c068c0a400a9 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Brown Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Matthew Mulder Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Tim Wood Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5413: Add a hive user for test seq writer hive compatibility.
David Knupp has posted comments on this change. Change subject: IMPALA-5413: Add a hive user for test_seq_writer_hive_compatibility. .. Patch Set 3: Code-Review+2 Removed one blank # comment. Carrying +2 from Alex. -- To view, visit http://gerrit.cloudera.org:8080/7046 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1cc8824800e4339874b9c4e3a84969baf848d941 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: David Knupp Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Lars Volker Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5413: Add a hive user for test seq writer hive compatibility.
David Knupp has posted comments on this change. Change subject: IMPALA-5413: Add a hive user for test_seq_writer_hive_compatibility. .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/7046/2/tests/query_test/test_compressed_formats.py File tests/query_test/test_compressed_formats.py: Line 182: # > remove? Done -- To view, visit http://gerrit.cloudera.org:8080/7046 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1cc8824800e4339874b9c4e3a84969baf848d941 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: David Knupp Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Lars Volker Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5413: Add a hive user for test seq writer hive compatibility.
Hello Alex Behm, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7046 to look at the new patch set (#3). Change subject: IMPALA-5413: Add a hive user for test_seq_writer_hive_compatibility. .. IMPALA-5413: Add a hive user for test_seq_writer_hive_compatibility. This patch includes a change to the framework to permit the passing of a username to the run_stmt_in_hive() method in the ImpalaTestSuite class, but retains the same default value as before. This is to allow a test to issue a 'select count(*) from foo' query through hive. Hive needs to set up a job to perform this query, and HDFS write access to do so. In typical cases, the HDFS user is 'hdfs'. however it may be necessary to change this depending on the cluster. On a local mini-cluster, the username appears to be irrelevant, so this won't affect locally run tests. Tested by running the core set of tests on a local minicluster to make sure there were no regressions. Also confirmed that the test in question now passes on a remote physical cluster. Change-Id: I1cc8824800e4339874b9c4e3a84969baf848d941 --- M tests/common/impala_test_suite.py M tests/query_test/test_compressed_formats.py 2 files changed, 10 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/46/7046/3 -- To view, visit http://gerrit.cloudera.org:8080/7046 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1cc8824800e4339874b9c4e3a84969baf848d941 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: David Knupp Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Lars Volker
[Impala-ASF-CR] IMPALA-5413: Add a hive user for test seq writer hive compatibility.
David Knupp has uploaded a new patch set (#2). Change subject: IMPALA-5413: Add a hive user for test_seq_writer_hive_compatibility. .. IMPALA-5413: Add a hive user for test_seq_writer_hive_compatibility. This patch includes a change to the framework to permit the passing of a username to the run_stmt_in_hive() method in the ImpalaTestSuite class, but retains the same default value as before. This is to allow a test to issue a 'select count(*) from foo' query through hive. Hive needs to set up a job to perform this query, and HDFS write access to do so. In typical cases, the HDFS user is 'hdfs'. however it may be necessary to change this depending on the cluster. On a local mini-cluster, the username appears to be irrelevant, so this won't affect locally run tests. Tested by running the core set of tests on a local minicluster to make sure there were no regressions. Also confirmed that the test in question now passes on a remote physical cluster. Change-Id: I1cc8824800e4339874b9c4e3a84969baf848d941 --- M tests/common/impala_test_suite.py M tests/query_test/test_compressed_formats.py 2 files changed, 11 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/46/7046/2 -- To view, visit http://gerrit.cloudera.org:8080/7046 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1cc8824800e4339874b9c4e3a84969baf848d941 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: David Knupp Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: David Knupp
[Impala-ASF-CR] IMPALA-5413: Skip test seq writer hive compatibility on remote clusters.
David Knupp has posted comments on this change. Change subject: IMPALA-5413: Skip test_seq_writer_hive_compatibility on remote clusters. .. Patch Set 1: It might be a bug in Hive or beeline? Beeline is throwing a WRITE access error on select count(*) from table, where impala-shell handles it fine. -- To view, visit http://gerrit.cloudera.org:8080/7046 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1cc8824800e4339874b9c4e3a84969baf848d941 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: David Knupp Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: David Knupp Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5455: test infra: propagate --cm-port, add --use-tls
David Knupp has posted comments on this change. Change subject: IMPALA-5455: test infra: propagate --cm-port, add --use-tls .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7107 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7dfa6c400687f3c5ccaf578fd4fb17dedd6eded Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Brown Gerrit-Reviewer: David Knupp Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5413: Skip test seq writer hive compatibility on remote clusters.
David Knupp has posted comments on this change.
Change subject: IMPALA-5413: Skip test_seq_writer_hive_compatibility on remote
clusters.
..
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/7046/1/tests/query_test/test_compressed_formats.py
File tests/query_test/test_compressed_formats.py:
Line 179: output = self.run_stmt_in_hive('select count(*) from %s' %
table_name)
> For clarification: We cannot really run any statements in Hive on a remote
Hmm, good point. It appears this is not the case -- we do have some other tests
in other modules that use this method. It must be something else.
Line 179: output = self.run_stmt_in_hive('select count(*) from %s' %
table_name)
> Just looked at the JIRA and it's not really clear to me what Hive is doing.
Thanks for looking at this Alex. I see what you mean. I'll try to figure out
why this is happening.
--
To view, visit http://gerrit.cloudera.org:8080/7046
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1cc8824800e4339874b9c4e3a84969baf848d941
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: David Knupp
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4482, IMPALA-4838: RECOVER PARTITIONS with tpcds.store sales
David Knupp has abandoned this change. Change subject: IMPALA-4482, IMPALA-4838: RECOVER PARTITIONS with tpcds.store_sales .. Abandoned Abandoning. Part of this change is not relevant anymore. I'll open a new review for the part that is. -- To view, visit http://gerrit.cloudera.org:8080/5177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: abandon Gerrit-Change-Id: Iaae97d1d44201aeeacacdd39adbae35753512950 Gerrit-PatchSet: 8 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: David Knupp Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Harrison Sheinblatt Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Michael Brown
[Impala-ASF-CR] IMPALA-5413: Skip test seq writer hive compatibility on remote clusters.
David Knupp has uploaded a new change for review. http://gerrit.cloudera.org:8080/7046 Change subject: IMPALA-5413: Skip test_seq_writer_hive_compatibility on remote clusters. .. IMPALA-5413: Skip test_seq_writer_hive_compatibility on remote clusters. There's no immediately obvious way to confirm that the user currently running the test will have the necessary access to the hive DB on a remote cluster, so this test will be intermittently flaky. Change-Id: I1cc8824800e4339874b9c4e3a84969baf848d941 --- M tests/query_test/test_compressed_formats.py 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/46/7046/1 -- To view, visit http://gerrit.cloudera.org:8080/7046 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I1cc8824800e4339874b9c4e3a84969baf848d941 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: David Knupp
[Impala-ASF-CR] Add a script to test performance on a developer machine
David Knupp has posted comments on this change. Change subject: Add a script to test performance on a developer machine .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/6818 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I70ba7f3c28f612a370915615600bf8dcebcedbc9 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Brown Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5366: datetime missing when upgrading to Ubuntu 16.04
David Knupp has posted comments on this change. Change subject: IMPALA-5366: datetime missing when upgrading to Ubuntu 16.04 .. Patch Set 1: Code-Review-1 -- To view, visit http://gerrit.cloudera.org:8080/6989 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I41501d273967dc1d57d366a145766a67b2dbbaa0 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Michael Brown Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5366: datetime missing when upgrading to Ubuntu 16.04
David Knupp has posted comments on this change. Change subject: IMPALA-5366: datetime missing when upgrading to Ubuntu 16.04 .. Patch Set 1: s/for that file/for that module/ -- To view, visit http://gerrit.cloudera.org:8080/6989 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I41501d273967dc1d57d366a145766a67b2dbbaa0 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: David Knupp Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5366: datetime missing when upgrading to Ubuntu 16.04
David Knupp has posted comments on this change.
Change subject: IMPALA-5366: datetime missing when upgrading to Ubuntu 16.04
..
Patch Set 1:
So, I'm a little concerned about this. The datetime being installed here isn't
the stdlib datetime. It's a thirdparty module from Zope. When you install it,
you also get other dependencies...
Installing collected packages: zope.interface, pytz, datetime
Successfully installed datetime-4.2 pytz-2017.2 zope.interface-4.4.1
And if you look at the comments in the setup.py for that file, this is included:
description="""\
This package provides a DateTime data type, as known from Zope 2.
Unless you need to communicate with Zope 2 APIs, you're probably
better off using Python's built-in datetime module.""".replace('\n', ' ')
https://github.com/zopefoundation/DateTime/blob/master/setup.py
--
To view, visit http://gerrit.cloudera.org:8080/6989
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I41501d273967dc1d57d366a145766a67b2dbbaa0
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: David Knupp
Gerrit-HasComments: No
[Impala-ASF-CR] Add a script to test performance on a developer machine
David Knupp has posted comments on this change. Change subject: Add a script to test performance on a developer machine .. Patch Set 4: (3 comments) http://gerrit.cloudera.org:8080/#/c/6818/4/bin/single_node_perf_run.py File bin/single_node_perf_run.py: Line 71: os.chdir(IMPALA_HOME) Given that you os.chdir(IMPALA_HOME) in main(), can it be removed elsewhere? Line 241: parser = OptionParser() Nit: I usually like to refactor the boilerplate option-parsing stuff to another function that returns the "options, args" tuple, and then call that from the main function. Personal preference though. Line 274: if not 1 <= len(args) <= 2: Seems confusing that 1 git hash is allowed when the docstrings and usage all say "Compares the performance of Impala at two git hashes..." -- To view, visit http://gerrit.cloudera.org:8080/6818 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I70ba7f3c28f612a370915615600bf8dcebcedbc9 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Brown Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5331: Use new libHDFS API to address "Unknown Error 255"
David Knupp has posted comments on this change. Change subject: IMPALA-5331: Use new libHDFS API to address "Unknown Error 255" .. Patch Set 7: Code-Review+1 Python test code looks OK to me. -- To view, visit http://gerrit.cloudera.org:8080/6894 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I181e316ed63b70b94d4f7a7557d398a931bb171d Gerrit-PatchSet: 7 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5331: Use new libHDFS API to address "Unknown Error 255"
David Knupp has posted comments on this change. Change subject: IMPALA-5331: Use new libHDFS API to address "Unknown Error 255" .. Patch Set 6: (2 comments) http://gerrit.cloudera.org:8080/#/c/6894/6/tests/data_errors/test_data_errors.py File tests/data_errors/test_data_errors.py: Line 78: try: I wonder about stacking all of these asserts in one block. If any of them fails, the test will obviously fail, but not before trying to "leave" safe mode in the finally: clause. Is that the right thing to do in every situation? (E.g., maybe it makes more sense to leave the state as-is for triage? I don't know, I'm just asking.) Also, presumably the hdfs commands could fail in multiple ways. The first check to "get" the safemode, for example -- either the hdfs command succeeds but safe mode is "ON", or the command itself fails for some other reason. To differentiate the behavior and provide better assistance for someone triaging test failures, you might consider examining stderr as well, using something like: proc = subprocess.Popen(['hdfs', 'dfsadmin', '-safemode', 'get'], stdout=subprocess.PIPE, stderr=subprocess.PIPE) output, error = proc.communicate() If "output" contains "Safe mode is ON" and "error" is an empty string, you know the command succeeded, but the expected state is incorrect. If the reverse is true, and "error" contains the meaningful string, then the command itself has failed. Perhaps you want to take different actions in each case. In the unlikely case that "hdfs" is not a valid command, an OSError will be raised. So maybe that's a third failure mode. Line 97: assert "Safe mode is OFF" in subprocess.Popen( If this is the happy path, this is fine. If the assert fails though, you might want to add a warning for the user that their safemode setting might be incorrect -- e.g., assert , "Command failed: system may still be in safemode" ... or something like that? -- To view, visit http://gerrit.cloudera.org:8080/6894 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I181e316ed63b70b94d4f7a7557d398a931bb171d Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5333: Add support for Impala to work with ADLS
David Knupp has posted comments on this change. Change subject: IMPALA-5333: Add support for Impala to work with ADLS .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/6910/2/tests/util/adls_util.py File tests/util/adls_util.py: Line 45: self.adlsclient.mkdir(path) > It looks like it just throws an exception: Nah. If you're not going to actually do anything with the exception, than no need to catch it. -- To view, visit http://gerrit.cloudera.org:8080/6910 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic56b9988b32a330443f24c44f9cb2c80842f7542 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5333: Add support for Impala to work with ADLS
David Knupp has posted comments on this change. Change subject: IMPALA-5333: Add support for Impala to work with ADLS .. Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/6910/2/tests/util/adls_util.py File tests/util/adls_util.py: Line 29: @classmethod This seems really unusual to me. __init__ methods typically don't need a @classmethod decorator, since what you're initializing is an instance of the class, not the class itself. By adding the decorator, "self" here becomes a reference to the class, not the instance. Perhaps there's something more exotic going on here that I'm not picking up on though. Let me know if I'm off target with this. I've just never seen this done. PS2, Line 40: f.close() > I believe this is redundant but I could be wrong. You're correct -- "with ... as foo" should close foo for you. Line 45: self.adlsclient.mkdir(path) What is the return value of self.adlsclient.mkdir(path)? Does it make sense to return it instead? What happens if the call fails? Line 72: files = self.ls(path) Super insignificant nit: you could put this inside of the list comprehension to make this method a tidy one-liner. -- To view, visit http://gerrit.cloudera.org:8080/6910 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic56b9988b32a330443f24c44f9cb2c80842f7542 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5333: Add support for Impala to work with ADLS
David Knupp has posted comments on this change.
Change subject: IMPALA-5333: Add support for Impala to work with ADLS
..
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/6910/2/tests/common/impala_test_suite.py
File tests/common/impala_test_suite.py:
Line 54: from tests.util.filesystem_utils import IS_S3, S3_BUCKET_NAME,
IS_ADLS, ADLS_STORE_NAME, \
This is a nit, but once the import list becomes this long, a common idiom is to
use...
from tests.util.filesystem_utils import (
IS_S3,
S3_BUCKET_NAME,
IS_ADLS,
ADLS_STORE_NAME,
...
)
Line 76: assert IS_ADLS == False, "Need the ADLSClient for testing with ADLS"
Is covering the ImportError with an AssertionError funky? Might it be better
for debugging if we do something like:
if IS_ADLS:
try:
from tests.util.adls_util import ADLSClient
except ImportError:
LOG.error("Need the ADLSClient for testing with ADLS")
raise
?
--
To view, visit http://gerrit.cloudera.org:8080/6910
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic56b9988b32a330443f24c44f9cb2c80842f7542
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil
Gerrit-Reviewer: Dan Hecht
Gerrit-Reviewer: David Knupp
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Matthew Jacobs
Gerrit-Reviewer: Sailesh Mukil
Gerrit-HasComments: Yes
[Impala-ASF-CR] Add a script to test performance on a developer machine
David Knupp has posted comments on this change. Change subject: Add a script to test performance on a developer machine .. Patch Set 2: (1 comment) Jim, this is mainly just to confirm that this review request has been noted. Will try to find time in the next couple of days to comment. http://gerrit.cloudera.org:8080/#/c/6818/2//COMMIT_MSG Commit Message: PS2, Line 20: buildall.sh Did you mean to add buildall.sh to this review? -- To view, visit http://gerrit.cloudera.org:8080/6818 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I70ba7f3c28f612a370915615600bf8dcebcedbc9 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Michael Brown Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5287: Test skip.header.line.count on gzip
David Knupp has posted comments on this change. Change subject: IMPALA-5287: Test skip.header.line.count on gzip .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/6817/1/tests/query_test/test_scanners.py File tests/query_test/test_scanners.py: PS1, Line 688: > Cool idea. I did this. Can you add this JIRA to the commit message? :-) https://issues.apache.org/jira/browse/IMPALA-4873 -- To view, visit http://gerrit.cloudera.org:8080/6817 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie3f3c29a42501cfb2751f7ad0af166eb88f63b70 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars Volker Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5189: Pin version of setuptools-scm
David Knupp has posted comments on this change. Change subject: IMPALA-5189: Pin version of setuptools-scm .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/6599 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I398972d2cdf3acc9d5d8c598fc5b964b7241f1d2 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars Volker Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5189: Pin version of setuptools scm
David Knupp has posted comments on this change. Change subject: IMPALA-5189: Pin version of setuptools_scm .. Patch Set 1: > unless we feel we should pin the package to 1.15.0 anyways We could pin to 1.15.4 as our known working version. -- To view, visit http://gerrit.cloudera.org:8080/6599 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I398972d2cdf3acc9d5d8c598fc5b964b7241f1d2 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars Volker Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Brown Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5189: Pin version of setuptools scm
David Knupp has posted comments on this change. Change subject: IMPALA-5189: Pin version of setuptools_scm .. Patch Set 1: > Do we still want to pin this dependency, or should we abandon this change? My opinion is that pinning dependencies to specific versions is generally a good practice. We've seen breakages from unpinned versions in other packages as well. -- To view, visit http://gerrit.cloudera.org:8080/6599 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I398972d2cdf3acc9d5d8c598fc5b964b7241f1d2 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars Volker Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Brown Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5181: Extract PYPI metadata from a webpage
David Knupp has posted comments on this change. Change subject: IMPALA-5181: Extract PYPI metadata from a webpage .. Patch Set 1: I'm totally in agreement. -- To view, visit http://gerrit.cloudera.org:8080/6579 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If3845a0d5f568d4352e3cc4883596736974fd7de Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Taras Bobrovytsky Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5181: Extract PYPI metadata from a webpage
David Knupp has posted comments on this change. Change subject: IMPALA-5181: Extract PYPI metadata from a webpage .. Patch Set 1: Again, just wanted to reiterate that I think if this is the quickest path to unlocking the repo, this is an acceptable approach. I don't mean to hold this up. Discussions as to whether we should revisit the whole virtualenv workflow can happen elsewhere. In light of that... 1. I think Lars' point about adding a comment in the code as to why lxml (etc.) can't be used is still a good one. 2. Can you add a testing comment in the commit msg describing how this was tested. Did you try it against the default public PYPI, and also non-default mirror? -- To view, visit http://gerrit.cloudera.org:8080/6579 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If3845a0d5f568d4352e3cc4883596736974fd7de Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Taras Bobrovytsky Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5181: Extract PYPI metadata from a webpage
David Knupp has posted comments on this change.
Change subject: IMPALA-5181: Extract PYPI metadata from a webpage
..
Patch Set 1:
There may well be some historical context that I'm missing here, or just some
complication that I'm overlooking, so sincere apologies if these comments are
not relevant, but one thing I've noticed is that even though we create a python
virtualenv, instead of ever activating it, we use wrapper scripts to construct
elaborate paths to the enclosed binaries (not just Impala does this, but other
Cloudera teams do this too.)
E.g., if I'm in my system evironment:
dknupp@dknupp-desktop:0:~ $ which python pip virtualenv
/usr/bin/python
/usr/local/bin/pip
/usr/local/bin/virtualenv
dknupp@dknupp-desktop:0:~ $ pip --version
pip 8.1.2 from /usr/local/lib/python2.7/dist-packages (python 2.7)
dknupp@dknupp-desktop:0:~ $ virtualenv --version
15.0.2
But if I _activate_ the Impala python virtualenv first...
dknupp@dknupp-desktop:0:~ $ source
${IMPALA_HOME}/infra/python/env/bin/activate
(env)dknupp@dknupp-desktop:0:~ $ which python pip virtualenv
/home/dknupp/Impala/infra/python/env/bin/python
/home/dknupp/Impala/infra/python/env/bin/pip
/home/dknupp/Impala/infra/python/env/bin/virtualenv
...the system versions no longer matter, and I can upgrade these at will.
(env)dknupp@dknupp-desktop:0:~ $ pip install -U pip virtualenv
Collecting pip
Using cached pip-9.0.1-py2.py3-none-any.whl
Collecting virtualenv
Downloading virtualenv-15.1.0-py2.py3-none-any.whl (1.8MB)
100% || 1.8MB 1.0MB/s
Installing collected packages: pip, virtualenv
Found existing installation: pip 8.1.2
Uninstalling pip-8.1.2:
Successfully uninstalled pip-8.1.2
Found existing installation: virtualenv 13.1.0
Uninstalling virtualenv-13.1.0:
Successfully uninstalled virtualenv-13.1.0
Successfully installed pip-9.0.1 virtualenv-15.1.0
(env)dknupp@dknupp-desktop:0:~ $ pip --version
pip 9.0.1 from
/home/dknupp/Impala/infra/python/env/local/lib/python2.7/site-packages (python
2.7)
(env)dknupp@dknupp-desktop:0:~ $ virtualenv --version
15.1.0
When I deactivate the virtualenv, the system versions are untouched.
dknupp@dknupp-desktop:0:~ $ pip --version
pip 8.1.2 from /usr/local/lib/python2.7/dist-packages (python 2.7)
dknupp@dknupp-desktop:0:~ $ virtualenv --version
15.0.2
So a possible workflow would be to:
1. Create the Impala virtualenv in ${IMPALA_HOME}/infra/python/env first
2. Activate the virtualenv
3. "pip install -U pip" to upgrade ${IMPALA_HOME}/infra/python/env/bin/pip
4. Using the upgraded pip to download & install the various things
5. Deactivate the virtualenv
--
To view, visit http://gerrit.cloudera.org:8080/6579
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If3845a0d5f568d4352e3cc4883596736974fd7de
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: David Knupp
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Taras Bobrovytsky
Gerrit-Reviewer: Tim Armstrong
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5181: Extract PYPI metadata from a webpage
David Knupp has posted comments on this change. Change subject: IMPALA-5181: Extract PYPI metadata from a webpage .. Patch Set 1: I have to confess, I've long felt like there's lot of over engineering in the way that we set up the Impala python environment. E.g., pip allows for this usage: pip download -i -r --no-binary=:all: It doesn't download wheels, and it won't download packages that are already present, which I think is the main purpose of this file? It might mean that we have to update our version of pip in infra/python/env/bin as a first step, but if it works, it would obviate the need for a lot of this logic where bugs can hide. If this is the fastest solution to getting build working again so we can unlock the repo and start checking in code again, that's fine. But I think that there might be an easier way to do this. -- To view, visit http://gerrit.cloudera.org:8080/6579 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If3845a0d5f568d4352e3cc4883596736974fd7de Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Taras Bobrovytsky Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5181: Extract PYPI metadata from a webpage
David Knupp has posted comments on this change. Change subject: IMPALA-5181: Extract PYPI metadata from a webpage .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/6579/1/infra/python/deps/pip_download.py File infra/python/deps/pip_download.py: Line 74: REQUIRED_PKG_TYPE = 'sdist' # Wheel archives are not supported. Perhaps a nit, but if we're considering REQUIRED_PKG_TYPE to be a "constant," I think I'd prefer to see it at the top of the file, rather than declared within this function. -- To view, visit http://gerrit.cloudera.org:8080/6579 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If3845a0d5f568d4352e3cc4883596736974fd7de Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Taras Bobrovytsky Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5079: Bump timeout for TestKuduOperations
David Knupp has posted comments on this change. Change subject: IMPALA-5079: Bump timeout for TestKuduOperations .. Patch Set 1: Code-Review+1 I agree -- I don't see any downsides to scoping the connection fixture at test level. -- To view, visit http://gerrit.cloudera.org:8080/6453 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5fd8e5eb5dbd977c386e6557941a2c47de44f344 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Michael Brown Gerrit-HasComments: No
[Impala-ASF-CR] Fix parameter order in test mt dop.py
David Knupp has posted comments on this change. Change subject: Fix parameter order in test_mt_dop.py .. Patch Set 1: Sure, for the sake of consistency, I'm OK with this change. There are actually two places where this occurs. tests/query_test/test_mt_dop.py: def test_compute_stats(self, unique_database, vector): tests/query_test/test_scanners.py: def test_resolution_by_name(self, unique_database, vector): -- To view, visit http://gerrit.cloudera.org:8080/6228 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I925711eb4d4334c179d336f6ebcc91d26ce8879c Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars Volker Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Matthew Jacobs Gerrit-HasComments: No
[Impala-ASF-CR] Fix parameter order in test mt dop.py
David Knupp has posted comments on this change. Change subject: Fix parameter order in test_mt_dop.py .. Patch Set 1: With pytest fixtures, I don't think order matters. -- To view, visit http://gerrit.cloudera.org:8080/6228 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I925711eb4d4334c179d336f6ebcc91d26ce8879c Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars Volker Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Matthew Jacobs Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4987: Skip test rows availability when testing over a network.
David Knupp has uploaded a new change for review. http://gerrit.cloudera.org:8080/6144 Change subject: IMPALA-4987: Skip test_rows_availability when testing over a network. .. IMPALA-4987: Skip test_rows_availability when testing over a network. Running this test against a remote cluster introduces network based timing irregularities that make it difficult to assert passing status. This is a test that only makes sense when the impalad is local. Change-Id: I4449d82ccd86fcf22ea96b36618c6de87458ffce --- M tests/query_test/test_rows_availability.py 1 file changed, 3 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/6144/1 -- To view, visit http://gerrit.cloudera.org:8080/6144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I4449d82ccd86fcf22ea96b36618c6de87458ffce Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: David Knupp
[Impala-ASF-CR] Add .pep8rc for Impala's Python style
David Knupp has posted comments on this change. Change subject: Add .pep8rc for Impala's Python style .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/5829/1/.pep8rc File .pep8rc: PS1, Line 2: # E101 - Reindent all lines. > Without E101 and E111, autopep8 will re-indent the whole file to use 4 spac For what it's worth, pursuant to the discussion on dev@, I don't think this is our one shot at this. As a first goal (setting up tooling), I think that ignoring this is probably OK. We can revisit later, right? PS1, Line 6: Add missing blank line Does making the description more verbose affect the functionality? "Expected 1 blank line, found 0. Add missing blank line." http://gerrit.cloudera.org:8080/#/c/5829/1//COMMIT_MSG Commit Message: PS1, Line 7: Add .pep8rc for Impala's Python style > No tools are automatically affected by this, but several can use it if poin I would appreciate having this pointed out here. Just speaking for myself, if I had questions about this file, I'd probably start by looking at the git log and read the commit messages before I searched on a wiki somewhere. -- To view, visit http://gerrit.cloudera.org:8080/5829 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I65a99fc6e364eb1e49e21d0ba97546bc25f65a27 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars Volker Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-HasComments: Yes
[Impala-ASF-CR] Three misc webpage changes
David Knupp has posted comments on this change. Change subject: Three misc webpage changes .. Patch Set 1: The changes here seem reasonable. Is there a plan to migrate other HTML tables to DataTables? (E.g., Queries, Query Locations, ...) -- To view, visit http://gerrit.cloudera.org:8080/6045 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ica0578dabb7e27e6fd45ee4f31a1418ac3adc891 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry Robinson Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Matthew Jacobs Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4904,IMPALA-4914: whitelist workload "targeted-stress" for exhaustive tests
David Knupp has posted comments on this change. Change subject: IMPALA-4904,IMPALA-4914: whitelist workload "targeted-stress" for exhaustive tests .. Patch Set 3: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/6002 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie6bd5bbd380e636d680368e908519b042d79dfec Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Brown Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR](asf-site) A blog post about IMPALA-4916
David Knupp has posted comments on this change. Change subject: A blog post about IMPALA-4916 .. Patch Set 3: Code-Review+1 Agreed. -- To view, visit http://gerrit.cloudera.org:8080/5995 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifa90bf5621ef6466a4821f77a6e8a8b20c5512ae Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: asf-site Gerrit-Owner: Jim Apple Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Brown Gerrit-HasComments: No
[Impala-ASF-CR](asf-site) A blog post about IMPALA-4916
David Knupp has posted comments on this change. Change subject: A blog post about IMPALA-4916 .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/5995/1/nikola_site_generator/posts/where-did-i-leave-my-keys.md File nikola_site_generator/posts/where-did-i-leave-my-keys.md: PS1, Line 43: SetsContainTheirElements > PS2 did change to underscores, but left two spaces. Works for me. -- To view, visit http://gerrit.cloudera.org:8080/5995 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifa90bf5621ef6466a4821f77a6e8a8b20c5512ae Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: asf-site Gerrit-Owner: Jim Apple Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Brown Gerrit-HasComments: Yes
[Impala-ASF-CR](asf-site) A blog post about IMPALA-4916
David Knupp has posted comments on this change. Change subject: A blog post about IMPALA-4916 .. Patch Set 3: (1 comment) So, I believe that all of the pages still point to the old blog site. This patch doesn't seem to address that just yet. Was that by intention? http://gerrit.cloudera.org:8080/#/c/5995/1/nikola_site_generator/requirements.txt File nikola_site_generator/requirements.txt: PS1, Line 9: > Good point; I have not. I have now gone through my list of general purpose I tried setting up a new environment using this requirements.txt, and nikola still seems to work fine. So I'm OK with this. -- To view, visit http://gerrit.cloudera.org:8080/5995 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifa90bf5621ef6466a4821f77a6e8a8b20c5512ae Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: asf-site Gerrit-Owner: Jim Apple Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Brown Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4904,IMPALA-4914: whitelist workload "targeted-stress" for exhaustive tests
David Knupp has posted comments on this change. Change subject: IMPALA-4904,IMPALA-4914: whitelist workload "targeted-stress" for exhaustive tests .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/6002/1/bin/run-all-tests.sh File bin/run-all-tests.sh: PS1, Line 95: around in many formats. Nit. I had to read this sentence a couple of times. How do you feel about this wording? "too large to load in all of the formats required by exhaustive mode." Or something like that. http://gerrit.cloudera.org:8080/#/c/6002/2/tests/custom_cluster/test_spilling.py File tests/custom_cluster/test_spilling.py: PS2, Line 57: super(CustomClusterTestSuite, cls).setup_class() What's the reason to not have this at the very top of the method? -- To view, visit http://gerrit.cloudera.org:8080/6002 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie6bd5bbd380e636d680368e908519b042d79dfec Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Brown Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR](asf-site) A blog post about IMPALA-4916
David Knupp has posted comments on this change. Change subject: A blog post about IMPALA-4916 .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/5995/1/nikola_site_generator/posts/where-did-i-leave-my-keys.md File nikola_site_generator/posts/where-did-i-leave-my-keys.md: PS1, Line 43: SetsContainTheirElements > I'v changed it to more closely match to code in tests/. To encourage develo I think cut-and-paste code from Impala could be a special case. FWIW, if I scan a blog post involving python code, and I see that the author hasn't followed commonly recognized PEP-8 conventions, it immediately colors how I read piece. Like, I immediately become a little skeptical. I'd at least change the function names to sets_contain_their_elements and change_keys, respectively. -- To view, visit http://gerrit.cloudera.org:8080/5995 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifa90bf5621ef6466a4821f77a6e8a8b20c5512ae Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: asf-site Gerrit-Owner: Jim Apple Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Brown Gerrit-HasComments: Yes
[Impala-ASF-CR](asf-site) A blog post about IMPALA-4916
David Knupp has posted comments on this change. Change subject: A blog post about IMPALA-4916 .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/5995/1/nikola_site_generator/posts/where-did-i-leave-my-keys.md File nikola_site_generator/posts/where-did-i-leave-my-keys.md: PS1, Line 50: s.add(set()) > This will throw an exception. You can't add a set to a set. Members of sets Sorry -- didn't read far enough. :-/ -- To view, visit http://gerrit.cloudera.org:8080/5995 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifa90bf5621ef6466a4821f77a6e8a8b20c5512ae Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: asf-site Gerrit-Owner: Jim Apple Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Brown Gerrit-HasComments: Yes
[Impala-ASF-CR](asf-site) A blog post about IMPALA-4916
David Knupp has posted comments on this change. Change subject: A blog post about IMPALA-4916 .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/5995/1/nikola_site_generator/posts/where-did-i-leave-my-keys.md File nikola_site_generator/posts/where-did-i-leave-my-keys.md: PS1, Line 43: SetsContainTheirElements Minor stylistic point. Let's follow strict PEP-8 for blog posts. PEP-8 calls for 4 space indents, stipulates that only classes are capitalized, and also advises against camel case for function names. PS1, Line 50: s.add(set()) This will throw an exception. You can't add a set to a set. Members of sets, like keys in dictionaries, need to be hashable, and so can't be mutable types. >>> s = set() >>> s.add(set()) Traceback (most recent call last): File "", line 1, in TypeError: unhashable type: 'set' -- To view, visit http://gerrit.cloudera.org:8080/5995 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifa90bf5621ef6466a4821f77a6e8a8b20c5512ae Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: asf-site Gerrit-Owner: Jim Apple Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Brown Gerrit-HasComments: Yes
[Impala-ASF-CR](asf-site) A blog post about IMPALA-4916
David Knupp has posted comments on this change. Change subject: A blog post about IMPALA-4916 .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/5995/1//COMMIT_MSG Commit Message: PS1, Line 7: A blog post about IMPALA-4916 It might be helpful to specify which files contain the actual article content, and which files were auto-generated by the framework. I.e., it might not be immediately obvious to people that the .md file is the main one to focus upon. http://gerrit.cloudera.org:8080/#/c/5995/1/nikola_site_generator/requirements.txt File nikola_site_generator/requirements.txt: PS1, Line 9: Have you tested that after removing this, nikola still works on a Mac? -- To view, visit http://gerrit.cloudera.org:8080/5995 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifa90bf5621ef6466a4821f77a6e8a8b20c5512ae Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: asf-site Gerrit-Owner: Jim Apple Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Brown Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4920: custom cluster tests: remove unnecessary escaped quote chars
David Knupp has posted comments on this change. Change subject: IMPALA-4920: custom cluster tests: remove unnecessary escaped quote chars .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/5978 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If6a56bc6bca826a8b50340e5caea7687441899e6 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Brown Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Michael Brown Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4839: Remove implicit 'localhost' for KUDU MASTER HOSTS
David Knupp has posted comments on this change. Change subject: IMPALA-4839: Remove implicit 'localhost' for KUDU_MASTER_HOSTS .. Patch Set 8: Pre-review build: http://jenkins.impala.io:8080/view/Utility/job/pre-review-test/18/ -- To view, visit http://gerrit.cloudera.org:8080/5877 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9df71480a165f4ce21ae3edab6ce7227fbf76f77 Gerrit-PatchSet: 8 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: David Knupp Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Ishaan Joshi Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Mostafa Mokhtar Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4839: Remove implicit 'localhost' for KUDU MASTER HOSTS
Hello Michael Brown, Matthew Jacobs, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5877 to look at the new patch set (#8). Change subject: IMPALA-4839: Remove implicit 'localhost' for KUDU_MASTER_HOSTS .. IMPALA-4839: Remove implicit 'localhost' for KUDU_MASTER_HOSTS The Kudu query tests were failing on a remote cluster because the Kudu master was always set to '127.0.0.1', with no way to override it. This patch corrects the issue with a number of changes: - Add a pytest command line option to specify an arbitrary Kudu master - Consolidate the place where the default Kudu master is derived. It had been stored both in the env and in tests/common/__init__.py, with different files looking to different places. For now, just look to the env, and remove the value from __init__.py. - The kudu_client test fixture in conftest.py was using the connect() method from impala.dbapi (part of the Impyla library), without specifying the host param. In the absence of that, the default value is 'localhost', so add the host param to the connect() call. - Define the various defaults for pytest config as constants at the top of conftest.py. Change-Id: I9df71480a165f4ce21ae3edab6ce7227fbf76f77 --- M bin/impala-config.sh M bin/start-impala-cluster.py M testdata/bin/generate-schema-statements.py M tests/common/__init__.py M tests/conftest.py M tests/custom_cluster/test_kudu.py M tests/query_test/test_kudu.py 7 files changed, 52 insertions(+), 39 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/77/5877/8 -- To view, visit http://gerrit.cloudera.org:8080/5877 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9df71480a165f4ce21ae3edab6ce7227fbf76f77 Gerrit-PatchSet: 8 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: David Knupp Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Ishaan Joshi Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Mostafa Mokhtar
[Impala-ASF-CR] IMPALA-4839: Remove implicit 'localhost' for KUDU MASTER HOSTS
David Knupp has posted comments on this change. Change subject: IMPALA-4839: Remove implicit 'localhost' for KUDU_MASTER_HOSTS .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/5877/7/tests/conftest.py File tests/conftest.py: Line 317: try: > With a yield fixture, py.test treats everything after the yield as teardown That's right. Thanks for catching this. I wasn't looking at anything outside of the scope of this particular problem. Even though it's orthogonal, I went ahead and fixed it, and checked that all of the Kudu tests still run. (Also, for what it's worth, we aren't upgrading past 2.9.2, since the next version of pytest following that introduced some backwards-breaking changes.) -- To view, visit http://gerrit.cloudera.org:8080/5877 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9df71480a165f4ce21ae3edab6ce7227fbf76f77 Gerrit-PatchSet: 7 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: David Knupp Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Ishaan Joshi Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Mostafa Mokhtar Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4839: Remove implicit 'localhost' for KUDU MASTER HOSTS
David Knupp has posted comments on this change. Change subject: IMPALA-4839: Remove implicit 'localhost' for KUDU_MASTER_HOSTS .. Patch Set 7: Code-Review+1 Carrying +1. -- To view, visit http://gerrit.cloudera.org:8080/5877 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9df71480a165f4ce21ae3edab6ce7227fbf76f77 Gerrit-PatchSet: 7 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: David Knupp Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Mostafa Mokhtar Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4839: Remove implicit 'localhost' for KUDU MASTER HOSTS
Hello Michael Brown, Matthew Jacobs, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5877 to look at the new patch set (#7). Change subject: IMPALA-4839: Remove implicit 'localhost' for KUDU_MASTER_HOSTS .. IMPALA-4839: Remove implicit 'localhost' for KUDU_MASTER_HOSTS The Kudu query tests were failing on a remote cluster because the Kudu master was always set to '127.0.0.1', with no way to override it. This patch corrects the issue with a number of changes: - Add a pytest command line option to specify an arbitrary Kudu master - Consolidate the place where the default Kudu master is derived. It had been stored both in the env and in tests/common/__init__.py, with different files looking to different places. For now, just look to the env, and remove the value from __init__.py. - The kudu_client test fixture in conftest.py was using the connect() method from impala.dbapi (part of the Impyla library), without specifying the host param. In the absence of that, the default value is 'localhost', so add the host param to the connect() call. - Define the various defaults for pytest config as constants at the top of conftest.py. Change-Id: I9df71480a165f4ce21ae3edab6ce7227fbf76f77 --- M bin/impala-config.sh M bin/start-impala-cluster.py M testdata/bin/generate-schema-statements.py M tests/common/__init__.py M tests/conftest.py M tests/custom_cluster/test_kudu.py M tests/query_test/test_kudu.py 7 files changed, 47 insertions(+), 33 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/77/5877/7 -- To view, visit http://gerrit.cloudera.org:8080/5877 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9df71480a165f4ce21ae3edab6ce7227fbf76f77 Gerrit-PatchSet: 7 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: David Knupp Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Mostafa Mokhtar
[Impala-ASF-CR] IMPALA-4839: Remove implicit 'localhost' for KUDU MASTER HOSTS
David Knupp has posted comments on this change. Change subject: IMPALA-4839: Remove implicit 'localhost' for KUDU_MASTER_HOSTS .. Patch Set 6: (2 comments) http://gerrit.cloudera.org:8080/#/c/5877/6//COMMIT_MSG Commit Message: PS6, Line 10: ovderride > override So weird. I was sure I changed that. Thanks. PS6, Line 12: change > changes Done -- To view, visit http://gerrit.cloudera.org:8080/5877 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9df71480a165f4ce21ae3edab6ce7227fbf76f77 Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: David Knupp Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Mostafa Mokhtar Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4839: Remove implicit 'localhost' for KUDU MASTER HOSTS
David Knupp has posted comments on this change. Change subject: IMPALA-4839: Remove implicit 'localhost' for KUDU_MASTER_HOSTS .. Patch Set 6: Pre-review build: http://jenkins.impala.io:8080/view/Utility/job/pre-review-test/17/ -- To view, visit http://gerrit.cloudera.org:8080/5877 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9df71480a165f4ce21ae3edab6ce7227fbf76f77 Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: David Knupp Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Mostafa Mokhtar Gerrit-HasComments: No
