Skip to content

Allow users to set hdfs_namenode_principal in HDFSHook config - #1112

Merged
bolkedebruin merged 1 commit into
apache:masterfrom
garthcn:hdfs-principal
Mar 19, 2016
Merged

Allow users to set hdfs_namenode_principal in HDFSHook config#1112
bolkedebruin merged 1 commit into
apache:masterfrom
garthcn:hdfs-principal

Conversation

@garthcn

@garthcn garthcn commented Mar 3, 2016

Copy link
Copy Markdown

snakebite library just added the support to specify hdfs_namenode_principal for Kerberos auth method, and this PR allows users to pass in this config from HDFSHook

@garthcn

garthcn commented Mar 3, 2016

Copy link
Copy Markdown
Author

Please review @mistercrunch @bolkedebruin
Thanks!

@bolkedebruin

Copy link
Copy Markdown
Contributor

@garthcn this won't be enough at it is dependent on a snakebite release. Is that new release available yet? If so then increase the requirements in setup.py. If not we have to wait :-)

@garthcn

garthcn commented Mar 3, 2016

Copy link
Copy Markdown
Author

@bolkedebruin snakebite has a new release https://pypi.python.org/pypi/snakebite/ and I just verified that it includes the PR I mentioned.

And in https://github.com/airbnb/airflow/blob/master/setup.py#L56, it has hdfs = ['snakebite>=2.4.13'] so it should install the latest snakebite :)

@bolkedebruin

Copy link
Copy Markdown
Contributor

But it won't install a new version. 2.7.8 is now required for existing installations. So it does need to be updated.

@garthcn

garthcn commented Mar 3, 2016

Copy link
Copy Markdown
Author

@bolkedebruin that makes sense. i just bumped the version in setup.py to the lastest (2.7.8) for snakebite

Comment thread airflow/hooks/hdfs_hook.py Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None is redundant/implicit here, no need to write it

@mistercrunch

Copy link
Copy Markdown
Member

@artwr, can you run our local tests against this for a final blessing?! Feel free to merge once you confirm it works.

@garthcn

garthcn commented Mar 8, 2016

Copy link
Copy Markdown
Author

@mistercrunch @artwr any update on this?

@artwr

artwr commented Mar 8, 2016

Copy link
Copy Markdown
Contributor

Sorry, I am a little tied up at the moment, but I promise I will get to it asap...

@garthcn

garthcn commented Mar 9, 2016

Copy link
Copy Markdown
Author

@artwr no worries. thanks for working on this :)

@garthcn

garthcn commented Mar 16, 2016

Copy link
Copy Markdown
Author

Ping :) @artwr

@bolkedebruin

Copy link
Copy Markdown
Contributor

@garthcn can you rebase? i will merge then

snakebite library just added the support to specify hdfs_namenode_principal
for Kerberos auth method, and this PR allows users to pass in this config from HDFSHook

Also bump the version of snakebite
@garthcn

garthcn commented Mar 18, 2016

Copy link
Copy Markdown
Author

@bolkedebruin just rebased to master and combined all commits.thanks!

@landscape-bot

Copy link
Copy Markdown

Code Health
Code quality remained the same when pulling 4af24ee on garthcn:hdfs-principal into ae8062a on airbnb:master.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.02%) to 63.508% when pulling 4af24ee on garthcn:hdfs-principal into ae8062a on airbnb:master.

bolkedebruin added a commit that referenced this pull request Mar 19, 2016
Allow users to set hdfs_namenode_principal in HDFSHook config
@bolkedebruin
bolkedebruin merged commit 697488e into apache:master Mar 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants