support federation and HA #22

Merged
merged 6 commits into from Jul 17, 2015

Conversation

Projects
None yet
2 participants
@cloverrose

Add new param path_to_hosts to PyWebHdfsClient constructor
to support federation and HA.

path_to_hosts is list of pair of path_regexp and hosts.
e.g. [('user/?.*', ['host1', 'host2']), ('.*', ['host3', 'host4'])]

Note: order is important.

in each function

  1. resolve federation:
    • Get hosts corresponds to path
  2. resolve HA:
    • There is one active host and others are standby host,
      so if failed with 1st host then try with 2nd host.
    • To improve efficiency, move active host to head of hosts.

relate issue #19

In my environment, standby host can treat CREATE operation and return valid init_response,
but I apply same logic to create_file function.

support federation and HA
Add new param path_to_hosts to PyWebHdfsClient constructor.

path_to_hosts is list of pair of path_regexp and hosts.
e.g. `[('user/?.*', ['host1', 'host2']), ('.*', ['host3', 'host4'])]`

Note: order is important.

in each function
1. resolve federation:
   Get hosts corresponds to path
2. resolve HA:
   There is one active host and others are standby host,
   so if failed with 1st host then try with 2nd host.
   To improve efficiency, move active host to head of hosts.
@cloverrose

This comment has been minimized.

Show comment
Hide comment
@cloverrose

cloverrose May 22, 2015

I would like to get your feedback.

I would like to get your feedback.

pywebhdfs/webhdfs.py
+ _move_active_host_to_head(hosts, host)
+ break
+ else:
+ raise errors.PyWebHdfsException(msg="Could not find active host")

This comment has been minimized.

@stevendgonzales

stevendgonzales Jul 10, 2015

I think we should add a custom exception so a user can catch it specifically rather than raise the general exception

@stevendgonzales

stevendgonzales Jul 10, 2015

I think we should add a custom exception so a user can catch it specifically rather than raise the general exception

This comment has been minimized.

@cloverrose

cloverrose Jul 10, 2015

I plan to make ActiveHostNotFound exception (relate HA) and CorrespondHostsNotFound exception (relate federation).

Then I want to know USECACE of _raise_pywebhdfs_exception().
And Should I use this method to raise above exceptions?

@cloverrose

cloverrose Jul 10, 2015

I plan to make ActiveHostNotFound exception (relate HA) and CorrespondHostsNotFound exception (relate federation).

Then I want to know USECACE of _raise_pywebhdfs_exception().
And Should I use this method to raise above exceptions?

pywebhdfs/webhdfs.py
+
+ hosts = self._resolve_federation(path)
+ for host in hosts:
+ uri = self._create_uri(host, path, operations.APPEND,

This comment has been minimized.

@stevendgonzales

stevendgonzales Jul 10, 2015

the code in the for loop is repetitive in nature for all these methods. Maybe there is a way to do some dynamic foo alleviate that? We don't necessarily have to do that for this PR.

@stevendgonzales

stevendgonzales Jul 10, 2015

the code in the for loop is repetitive in nature for all these methods. Maybe there is a way to do some dynamic foo alleviate that? We don't necessarily have to do that for this PR.

pywebhdfs/webhdfs.py
+ uri = self._create_uri(host, path, operations.CREATE,
+ **optional_args)
+ init_response = requests.put(uri, allow_redirects=False)
+ if not _is_standby_exception(init_response):

This comment has been minimized.

@stevendgonzales

stevendgonzales Jul 10, 2015

not for this PR, but I'm thinking a log handler really needs to be added so that we can emit a log message when the namenode is failing over

@stevendgonzales

stevendgonzales Jul 10, 2015

not for this PR, but I'm thinking a log handler really needs to be added so that we can emit a log message when the namenode is failing over

@stevendgonzales

This comment has been minimized.

Show comment
Hide comment
@stevendgonzales

stevendgonzales Jul 10, 2015

Sorry it took me so long to look at this. I made some small comments but am happy to merge. Let me know if you want to make changes here or are interested in adding another PR?

Sorry it took me so long to look at this. I made some small comments but am happy to merge. Let me know if you want to make changes here or are interested in adding another PR?

@cloverrose

This comment has been minimized.

Show comment
Hide comment
@cloverrose

cloverrose Jul 10, 2015

Thank you for your comments.
I'll make some changes here.

Thank you for your comments.
I'll make some changes here.

@cloverrose

This comment has been minimized.

Show comment
Hide comment
@cloverrose

cloverrose Jul 17, 2015

@stevendgonzales I made 2 changes (custom exceptions and extract method).
(I also think adding log handler is out of the PR.)

@stevendgonzales I made 2 changes (custom exceptions and extract method).
(I also think adding log handler is out of the PR.)

@stevendgonzales

This comment has been minimized.

Show comment
Hide comment

Thank you!

stevendgonzales added a commit that referenced this pull request Jul 17, 2015

@stevendgonzales stevendgonzales merged commit e3cc58f into pywebhdfs:master Jul 17, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment