Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug fixes for Hive CLI/Beeline compatibility #1616

Merged
merged 2 commits into from
Apr 4, 2016
Merged

Bug fixes for Hive CLI/Beeline compatibility #1616

merged 2 commits into from
Apr 4, 2016

Conversation

PeteW
Copy link
Contributor

@PeteW PeteW commented Mar 30, 2016

when you use beeline instead of hive you can specify hive command in luigi config file butyour command may have multiple options. For this reason the load_hive_cmd() method should return an array and not a single string.
When your task has parameters, [self.task_id] has parenthesis and this will cause shell.Popen issues (OsError). Fixed by wrapping [self.task_id] in quotes.

@Tarrasch
Copy link
Contributor

Thanks this looks good....

Except for the ALL CAPS COMMIT MESSAGE WHICH WILL BE EXTREMELY DISTRACTING FOR PEOPLE LOOKING IN THE COMMIT LOG.

Please amend you commit. After that this can be merged. :)

when you use beeline instead of hive you can specify hive command in luigi config file butyour command may have multiple options. For this reason the load_hive_cmd() method should return an array and not a single string.
When your task has parameters, [self.task_id] has parenthesis and this will cause shell.Popen issues (OsError). Fixed by wrapping [self.task_id] in quotes.
@PeteW PeteW changed the title BUG FIXES FOR HIVE CLI/BEELINE COMPATIBILITY Bug fixes for Hive CLI/Beeline compatibility Mar 31, 2016
@PeteW
Copy link
Contributor Author

PeteW commented Mar 31, 2016

@Tarrasch THE COMMIT MESSAGE HAS BEEN AMENDED PER YOUR REQUEST. SORRY ABOUT THAT. I HAVE TROUBLE WITH TYPING LOUD SOMETIMES

@@ -297,7 +297,7 @@ def hiveconfs(self):
* hive.exec.reducers.max (reducers_max)
"""
jcs = {}
jcs['mapred.job.name'] = self.task_id
jcs['mapred.job.name'] = "'" + self.task_id + "'"
Copy link
Contributor

Choose a reason for hiding this comment

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

"'%s'" % self.task_id would have been marginally more pythonic, but not blocking

@erikbern
Copy link
Contributor

LOOKS GOOD TO ME

@Tarrasch
Copy link
Contributor

Tarrasch commented Apr 1, 2016

Sorry to ask for another round-trip. But can you also remove the useless merge commit in this PR? It should only be 1 commit.

@Tarrasch
Copy link
Contributor

Tarrasch commented Apr 4, 2016

Actually no need! I can now squash commits directly from the web browser! Finally!! 🎉 🎉 🎉

https://github.com/blog/2141-squash-your-commits

@Tarrasch Tarrasch merged commit 744febe into spotify:master Apr 4, 2016
@PeteW
Copy link
Contributor Author

PeteW commented Apr 4, 2016

Cool!
I didnt know how to kill this commit from the command line :(
I spent a solid 30 seconds researching it before I saw a shiny object and
forgot to follow through for you.

BTW I love your work. I also found more "issues" maybe bugs? maybe I am the
bug? I am not sure but after I get some more practice I may have
suggestions around how "outputs" and "targets" are handled from classes
that inherit from BaseHadoopJobTask.

On 4 April 2016 at 00:14, Arash Rouhani notifications@github.com wrote:

Actually no need! I can now squash commits directly from the web browser!
Finally!! [image: 🎉] [image: 🎉] [image: 🎉]

https://github.com/blog/2141-squash-your-commits


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#1616 (comment)

Peter Weissbrod
http://RealLifeData.com | O: 207.439.3489 | M: 406.531.1263

@Tarrasch
Copy link
Contributor

Tarrasch commented Apr 5, 2016

Haha. :)

Please send pull requests if you find any more bugs! That's the community spirit that keeps the code base up-to-date!

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.

None yet

3 participants