You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Dan Hecht (Code Review)" <ge...@cloudera.org> on 2018/06/01 17:25:20 UTC
[Impala-ASF-CR] IMPALA-7109: fix test multiple partitions same location paths
Dan Hecht has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10575
Change subject: IMPALA-7109: fix test_multiple_partitions_same_location paths
......................................................................
IMPALA-7109: fix test_multiple_partitions_same_location paths
The paths passed to filesystem_util methods is bogus in this test,
but it's benign since we don't need to be doing these filesystem
operations manually for this managed table. Just delete these lines
(fixing them would create the same problem as IMPALA-7099).
Change-Id: I7fc0b2aa2e82bfd8224c52546683f23de20cb640
---
M tests/metadata/test_partition_metadata.py
1 file changed, 1 insertion(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/10575/1
--
To view, visit http://gerrit.cloudera.org:8080/10575
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7fc0b2aa2e82bfd8224c52546683f23de20cb640
Gerrit-Change-Number: 10575
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Hecht <dh...@cloudera.com>
[Impala-ASF-CR] IMPALA-7109: fix test multiple partitions same location paths
Posted by "Sailesh Mukil (Code Review)" <ge...@cloudera.org>.
Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/10575 )
Change subject: IMPALA-7109: fix test_multiple_partitions_same_location paths
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10575/1/tests/metadata/test_partition_metadata.py
File tests/metadata/test_partition_metadata.py:
http://gerrit.cloudera.org:8080/#/c/10575/1/tests/metadata/test_partition_metadata.py@52
PS1, Line 52:
> Done
Thanks for checking!
--
To view, visit http://gerrit.cloudera.org:8080/10575
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7fc0b2aa2e82bfd8224c52546683f23de20cb640
Gerrit-Change-Number: 10575
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Jun 2018 18:29:54 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7109: fix test multiple partitions same location paths
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10575 )
Change subject: IMPALA-7109: fix test_multiple_partitions_same_location paths
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/10575
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7fc0b2aa2e82bfd8224c52546683f23de20cb640
Gerrit-Change-Number: 10575
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Jun 2018 17:43:22 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7109: fix test multiple partitions same location paths
Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/10575 )
Change subject: IMPALA-7109: fix test_multiple_partitions_same_location paths
......................................................................
Patch Set 3: Code-Review+2
Carry
--
To view, visit http://gerrit.cloudera.org:8080/10575
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7fc0b2aa2e82bfd8224c52546683f23de20cb640
Gerrit-Change-Number: 10575
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Jun 2018 16:54:11 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7109: fix test multiple partitions same location paths
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10575 )
Change subject: IMPALA-7109: fix test_multiple_partitions_same_location paths
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/10575
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7fc0b2aa2e82bfd8224c52546683f23de20cb640
Gerrit-Change-Number: 10575
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Jun 2018 20:24:47 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7109: fix test multiple partitions same location paths
Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/10575 )
Change subject: IMPALA-7109: fix test_multiple_partitions_same_location paths
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10575/1/tests/metadata/test_partition_metadata.py
File tests/metadata/test_partition_metadata.py:
http://gerrit.cloudera.org:8080/#/c/10575/1/tests/metadata/test_partition_metadata.py@52
PS1, Line 52: qualified_path
> I think it was originally added for tests that explicitly used locations li
Yeah, you're right, now I see the reference to IMPALA-1872 in the non-local skip markers.
I think i'll revert this back to hdfs_client but add a TODO saying it's probably not actually dependent on that anymore.
--
To view, visit http://gerrit.cloudera.org:8080/10575
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7fc0b2aa2e82bfd8224c52546683f23de20cb640
Gerrit-Change-Number: 10575
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Jun 2018 18:05:23 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7109: fix test multiple partitions same location paths
Posted by "Sailesh Mukil (Code Review)" <ge...@cloudera.org>.
Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/10575 )
Change subject: IMPALA-7109: fix test_multiple_partitions_same_location paths
......................................................................
Patch Set 1:
(1 comment)
Was looking at IMPALA-6119, so had a look at this since it's somewhat related. Fix makes sense, just one question.
http://gerrit.cloudera.org:8080/#/c/10575/1/tests/metadata/test_partition_metadata.py
File tests/metadata/test_partition_metadata.py:
http://gerrit.cloudera.org:8080/#/c/10575/1/tests/metadata/test_partition_metadata.py@52
PS1, Line 52: qualified_path
Why 'qualified_path' ? Wouldn't TBL_LOCATION work for local as well since WAREHOUSE would point to the appropriate local warehouse location?
--
To view, visit http://gerrit.cloudera.org:8080/10575
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7fc0b2aa2e82bfd8224c52546683f23de20cb640
Gerrit-Change-Number: 10575
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Jun 2018 17:39:09 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7109: fix test multiple partitions same location paths
Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/10575 )
Change subject: IMPALA-7109: fix test_multiple_partitions_same_location paths
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10575/1/tests/metadata/test_partition_metadata.py
File tests/metadata/test_partition_metadata.py:
http://gerrit.cloudera.org:8080/#/c/10575/1/tests/metadata/test_partition_metadata.py@52
PS1, Line 52: qualified_path
> Why 'qualified_path' ? Wouldn't TBL_LOCATION work for local as well since W
What does the qualified_path skip marker mean then, if not that we can't use qualified paths on local?
But yeah, It seems like it should work. I can try removing this, though not sure it's worth the time.
--
To view, visit http://gerrit.cloudera.org:8080/10575
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7fc0b2aa2e82bfd8224c52546683f23de20cb640
Gerrit-Change-Number: 10575
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Jun 2018 17:41:03 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7109: fix test multiple partitions same location paths
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10575 )
Change subject: IMPALA-7109: fix test_multiple_partitions_same_location paths
......................................................................
IMPALA-7109: fix test_multiple_partitions_same_location paths
The paths passed to filesystem_util methods is bogus in this test,
but it's benign since we don't need to be doing these filesystem
operations manually for this managed table. Just delete these lines
(fixing them would create the same problem as IMPALA-7099).
Change-Id: I7fc0b2aa2e82bfd8224c52546683f23de20cb640
Reviewed-on: http://gerrit.cloudera.org:8080/10575
Reviewed-by: Dan Hecht <dh...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M tests/metadata/test_partition_metadata.py
1 file changed, 1 insertion(+), 7 deletions(-)
Approvals:
Dan Hecht: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/10575
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I7fc0b2aa2e82bfd8224c52546683f23de20cb640
Gerrit-Change-Number: 10575
Gerrit-PatchSet: 4
Gerrit-Owner: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-7109: fix test multiple partitions same location paths
Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Hello Sailesh Mukil, Tim Armstrong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10575
to look at the new patch set (#2).
Change subject: IMPALA-7109: fix test_multiple_partitions_same_location paths
......................................................................
IMPALA-7109: fix test_multiple_partitions_same_location paths
The paths passed to filesystem_util methods is bogus in this test,
but it's benign since we don't need to be doing these filesystem
operations manually for this managed table. Just delete these lines
(fixing them would create the same problem as IMPALA-7099).
Change-Id: I7fc0b2aa2e82bfd8224c52546683f23de20cb640
---
M tests/metadata/test_partition_metadata.py
1 file changed, 1 insertion(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/10575/2
--
To view, visit http://gerrit.cloudera.org:8080/10575
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7fc0b2aa2e82bfd8224c52546683f23de20cb640
Gerrit-Change-Number: 10575
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-7109: fix test multiple partitions same location paths
Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/10575 )
Change subject: IMPALA-7109: fix test_multiple_partitions_same_location paths
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10575/1/tests/metadata/test_partition_metadata.py
File tests/metadata/test_partition_metadata.py:
http://gerrit.cloudera.org:8080/#/c/10575/1/tests/metadata/test_partition_metadata.py@52
PS1, Line 52: qualified_path
> Yeah, you're right, now I see the reference to IMPALA-1872 in the non-local
Done
--
To view, visit http://gerrit.cloudera.org:8080/10575
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7fc0b2aa2e82bfd8224c52546683f23de20cb640
Gerrit-Change-Number: 10575
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Jun 2018 18:29:27 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7109: fix test multiple partitions same location paths
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10575 )
Change subject: IMPALA-7109: fix test_multiple_partitions_same_location paths
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2627/
--
To view, visit http://gerrit.cloudera.org:8080/10575
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7fc0b2aa2e82bfd8224c52546683f23de20cb640
Gerrit-Change-Number: 10575
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Jun 2018 16:54:16 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7109: fix test multiple partitions same location paths
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10575 )
Change subject: IMPALA-7109: fix test_multiple_partitions_same_location paths
......................................................................
Patch Set 2:
Seems ok to merge this now.
--
To view, visit http://gerrit.cloudera.org:8080/10575
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7fc0b2aa2e82bfd8224c52546683f23de20cb640
Gerrit-Change-Number: 10575
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Jun 2018 16:50:27 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7109: fix test multiple partitions same location paths
Posted by "Sailesh Mukil (Code Review)" <ge...@cloudera.org>.
Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/10575 )
Change subject: IMPALA-7109: fix test_multiple_partitions_same_location paths
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10575/1/tests/metadata/test_partition_metadata.py
File tests/metadata/test_partition_metadata.py:
http://gerrit.cloudera.org:8080/#/c/10575/1/tests/metadata/test_partition_metadata.py@52
PS1, Line 52: qualified_path
> What does the qualified_path skip marker mean then, if not that we can't us
I think it was originally added for tests that explicitly used locations like "hdfs://...", since those tests weren't expected to work on non HDFS filesystems.
But I agree, it's probably a waste of time trying to get this working for local now. LGTM otherwise.
--
To view, visit http://gerrit.cloudera.org:8080/10575
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7fc0b2aa2e82bfd8224c52546683f23de20cb640
Gerrit-Change-Number: 10575
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Jun 2018 17:50:51 +0000
Gerrit-HasComments: Yes