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.

Sign up for GitHub

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

Jump to bottom

HA enabled ResourceManager compatible #4

Closed
wants to merge 13 commits into from
Closed

HA enabled ResourceManager compatible #4

wants to merge 13 commits into from

Conversation

xiaop1987
Copy link

  1. When ResourceManager enabled ha, the older way can't get the resource manager correct.
  2. When reusing the HttpConnection, the request will fail raising a ResponseNotReady exception.

@ediskandarov
Copy link
Collaborator

Thanks for pull request @xiaop1987

Can you fix tests?

顾添锦 added 2 commits June 25, 2016 16:35
Remove testing cache case
18576d0
1. Fix case of test_hadoop_conf.HadoopConfTestCase.test_get_resource_…
3646a87 
…host_port

2. Add case of test_hadoop_conf.HadoopConfTestCase.test_get_resource_host_port_with_ha
@coveralls
Copy link

coveralls commented Jun 25, 2016 •
edited
Loading

Coverage Status

Coverage decreased (-2.08%) to 93.75% when pulling 3646a87 on xiaop1987:master into 9c13c50 on toidi:master.

@xiaop1987
Copy link
Author

xiaop1987 commented Jun 25, 2016 •
edited
Loading

@toidi I passed all the test cases in my environment, not quite understand the fail of Travis CI, the coverage is not good enough, i'll add some cases later.

@coveralls
Copy link

coveralls commented Jun 25, 2016 •
edited
Loading

Coverage Status

Coverage decreased (-1.9%) to 93.945% when pulling d61906a on xiaop1987:master into 9c13c50 on toidi:master.

@ediskandarov
Copy link
Collaborator

@xiaop1987 I think we can drop support to python 3.2 and add tests for python 3.4, python 3.5.
It will fix tests.

@coveralls
Copy link

coveralls commented Jun 28, 2016 •
edited
Loading

Coverage Status

Coverage decreased (-1.9%) to 93.945% when pulling 54aa445 on xiaop1987:master into 9c13c50 on toidi:master.

@coveralls
Copy link

coveralls commented Jun 28, 2016 •
edited
Loading

Coverage Status

Coverage decreased (-1.9%) to 93.945% when pulling 54aa445 on xiaop1987:master into 9c13c50 on toidi:master.

@xiaop1987
Copy link
Author

@toidi All cases should pass now.

ediskandarov
ediskandarov reviewed Jun 28, 2016
View reviewed changes
yarn_api_client/hadoop_conf.py
return rm_ids


def get_resource_manager(hadoop_conf_path, rm_id = None):
Copy link
Collaborator

Choose a reason for hiding this comment

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

If get_resource_manager is a function for internal usage - prefix it with underscore.

1. Add some case to increase coverage
bbf0ed5 
2. Fix the private method name style in hadoop_conf.py
@coveralls
Copy link

coveralls commented Jun 28, 2016 •
edited
Loading

Coverage Status

Coverage decreased (-0.9%) to 94.922% when pulling bbf0ed5 on xiaop1987:master into 9c13c50 on toidi:master.

@coveralls
Copy link

coveralls commented Jun 28, 2016 •
edited
Loading

Coverage Status

Coverage decreased (-0.9%) to 94.922% when pulling 4936f15 on xiaop1987:master into 9c13c50 on toidi:master.

@coveralls
Copy link

coveralls commented Jun 28, 2016 •
edited
Loading

Coverage Status

Coverage increased (+0.3%) to 96.094% when pulling dad8524 on xiaop1987:master into 9c13c50 on toidi:master.

@xiaop1987
Copy link
Author

Hi @toidi , the method name issue you mentioned has been fixed, I add some cases to increase the coverage.

@ediskandarov
Copy link
Collaborator

@xiaop1987 good job 👍

Squash commits, bump version and I'll merge and update PyPI package.

tianjin.gutj added 2 commits June 29, 2016 10:21
1. Reusing HttpConnection cause ResponseNotReady(see also http://stac…
648c19e 
…koverflow.com/questions/3231543/python-httplib-responsenotready), fix this bug

2. Make compatible to resource manager which ha enabled
3. Add cases for new feature
4. Remove run tests in py32 py33 and add py34 py35
5. Bump version to 0.2.4
Merge remote-tracking branch 'origin/master'
70288df
@coveralls
Copy link

coveralls commented Jun 29, 2016 •
edited
Loading

Coverage Status

Coverage increased (+0.3%) to 96.094% when pulling 70288df on xiaop1987:master into 9c13c50 on toidi:master.

@xiaop1987
Copy link
Author

@toidi I squashed the commits, but don't known did it correctly. Check it please.

@ediskandarov
Copy link
Collaborator

@xiaop1987 sorry for the delay. I was quite busy last days.

I'll merge it tonight.

@xiaop1987
Copy link
Author

@toidi no need to sorry at all, merge it whenever you are free :), your project here saved me lots of time, I'm very appreciate about that.

@xiaop1987 xiaop1987 mentioned this pull request Jul 8, 2016
Merged
@ediskandarov
Copy link
Collaborator

@xiaop1987 it's not what I expected. I'll create another PR with correct squashing

@ediskandarov ediskandarov mentioned this pull request Jul 10, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
duplicate
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@xiaop1987 @ediskandarov @coveralls