make hive macros return string type vs bytes

[Acehaidrey]

With the current implementation of the hive macros encoding the resultant from the metastore calls, in py2 this returns a string type still but in python3 encoding forces the representation to be a byte type. See the example below

ahaidrey-078HTD6:incubator-airflow ahaidrey$ python3
Python 3.7.0 (default, Oct  2 2018, 09:20:07)
[Clang 10.0.0 (clang-1000.11.45.2)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> a = 'apple'
>>> b = a.encode('utf-8')
>>> type(b)
<class 'bytes'>

ahaidrey-078HTD6:~ ahaidrey$ python
Python 2.7.16 (default, Dec  3 2019, 02:03:47)
[GCC 4.2.1 Compatible Apple LLVM 9.0.0 (clang-900.0.31)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> a = 'apple'
>>> b = a.encode('utf-8')
>>> type(b)
<type 'str'>

The issue with this is that the resultant for example being used by macros returns a byte type that isn't templatable as a string and breaks the queries it is used in. What this means is that all the templates need to be written as something like this:

val = "{{ macros.hive.max_partition(table='mytable', schema='myschema', field='myfield', filter_map={'key1': 'val1'}).decode('utf-8') }}"

Requiring from the users end to always decode the value is not the intention of this method and should use a value that can be returned as is.

This PR is to fix this ordeal. We may be able to just remove the encoding altogether but it could make things backwards incompatible.

---

Make sure to mark the boxes below before creating PR: [x]

• Description above provides context of the change
• Unit tests coverage for changes (not needed for documentation changes)
• Target Github ISSUE in description if exists (not existing)
• Commits follow "How to write a good git commit message"
• Relevant documentation is updated including usage instructions.
• I will engage committers as explained in Contribution Workflow Example.

---

In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.
Read the Pull Request Guidelines for more information.

[jhtimmins]

It looks like (at least some of) the tests are failing because other tests that call . _get_max_partition_from_part_specs() expect to get a byte value back, whereas now it's returning a string. See line 296 of test_hive.py.

[Acehaidrey]

The reason is that the consumers of this return type should not be getting a byte back but a real string response. Wanted to make this clear

[jhtimmins]

@Acehaidrey Not sure I have the context to say.

Could you explain what you mean by "incompatible return"?

[Acehaidrey]

Hi @jhtimmins ,

Sorry for the delayed response. I cleaned my message - realized it didn't make sense. There is not any incompatible return. But I fixed the tests so please take a look. This method is intended to be used with passing string values to scripts in template-able sql scripts and returning a string type is what is expected.

[Acehaidrey]

@jhtimmins if you have a chance to revisit this

[Acehaidrey]

@jhtimmins sorry for pestering but would love to get these in and close out

[Acehaidrey]

@ashb mind taking a look at this one too? sorry for all the tags

[Acehaidrey]

@ashb mind taking a look now? did the updates

[Acehaidrey]

@turbaszek mind taking a look at this too?

[Acehaidrey]

@turbaszek I just rebased instead of git pull etc. @ashb mind please taking a look one last time to close this out once and for all?

[ashb]

This looks okay for master, any idea what the change should look like for 1.10 where we still have to support py2+3

[Acehaidrey]

Hey @ashb for 1.10 it actually can remain the same. So from a quick test I ran locally even if py2 it returns the value as a string type. Not sure why they encoded but maybe older versions of py2 had some caveat?

Also thanks for reviewing!

[Acehaidrey]

hey @ashb mind taking a look at the latest comment.

[ashb]

Hey @ashb for 1.10 it actually can remain the same. So from a quick test I ran locally even if py2 it returns the value as a string type. Not sure why they encoded but maybe older versions of py2 had some caveat?

Also thanks for reviewing!

What about py3 on 1.10.x?

[Acehaidrey]

This change is actually done using branch v1.10-stable. So it works as is here @ashb . Good question. So no concern there either

[Acehaidrey]

@ashb wanted too knoow if there were any more concerns or if we merge this

[Acehaidrey]

@ashb sorry to keep pinging -

[Acehaidrey]

@ashb @turbaszek any chance we can get this in

[ashb]

I'll look first thing tomorrow morning. It should be good!

(Sorry, we've had some issues with our ci that need attention)

[Acehaidrey]

thank team!