impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Brown (Code Review)" <ger...@cloudera.org>
Subject [Impala-CR](cdh5-trunk) IMPALA-2650: UDF EE tests: use unique databases in some tests
Date Thu, 24 Mar 2016 00:35:37 GMT
Michael Brown has posted comments on this change.

Change subject: IMPALA-2650: UDF EE tests: use unique databases in some tests
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/2610/2/tests/query_test/test_udfs.py
File tests/query_test/test_udfs.py:

Line 179:     drop_fn_template = 'drop function if exists `{0}`.`%s`()'.format(unique_database)
> I think it's really weird to mix {0} and %s.
I don't think there are better ways to do it in Python. I can't speak to the original author's
intent, but I think these templates exists so the database variable doesn't have to be written
too much. Doing what you suggest kind of defeats the purpose of the template entirely. I tried
to keep the spirit of that.

For a crazier example, look at L185-187.

One option is to use {{0}} to create a format option and keep the template.

Another possible way to make it a little more clear of the templating nature of the strings
is to make them lambda expressions.

Which is worse:

  >>> template1 = '{0} %s {1}'.format('foo', 'bar')
  >>> print template1 % 'baz'
  foo baz bar
  >>> 

vs.

  >>> template2 = '{0} {{0}} {1}'.format('foo', 'bar')
  >>> print template2.format('baz')
  foo baz bar
  >>> 

While template1 mixes formatting, it might make things clearer than template2. Let me know
what you think.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4a8b4764fa310efaa4f6c6f06f64a4e18e44173
Gerrit-PatchSet: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Michael Brown <mikeb@cloudera.com>
Gerrit-Reviewer: Michael Brown <mikeb@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tbobrovytsky@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message