impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Lars Volker (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5009: Clean up test insert parquet.py
Date Thu, 27 Jul 2017 19:02:15 GMT
Lars Volker has posted comments on this change.

Change subject: IMPALA-5009: Clean up test_insert_parquet.py
......................................................................


Patch Set 1:

(2 comments)

Thank you for the reviews.

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 345: tmpdir
> Nit: for these private helper methods, prefer a more descriptive name than 
The folder is really not more than a temporary storage place that the caller has to provide
and clean up. It's not used to return any files back to the caller. I couldn't think of a
better name so I renamed it to tmp_dir to follow the naming convention and added a sentence
to the docstrings to explain what it is used for and that the caller is responsible for cleaning
it up. I'm happy to rename it though. Let me know if you prefer a different name.


PS1, Line 351: tmpdir.strpath
> +1 to this idea.
Done


-- 
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 <lv@cloudera.com>
Gerrit-Reviewer: David Knupp <dknupp@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv@cloudera.com>
Gerrit-Reviewer: Michael Brown <mikeb@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message