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

Add 2.1 Preflists and Write-Once Bucket Types #414

Merged
merged 8 commits into from Oct 21, 2015
Merged

Conversation

hazen
Copy link

@hazen hazen commented Oct 11, 2015

These features added in Riak 2.1 have been missing from the Python client up until now.

@hazen
Copy link
Author

hazen commented Oct 11, 2015

FYI - Need to update the riak_pb distribution before this will work

@hazen
Copy link
Author

hazen commented Oct 17, 2015

Determined that this will fail on 2.1.1 because it is missing @lukebakken's fix for preflist permissions: basho/riak_kv#1116


:rtype: bool
"""
return self.server_version >= versions[2.1]
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this also check for security and, if enabled, require a version greater than 2.1.1 ?

Copy link
Author

Choose a reason for hiding this comment

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

That's a great idea. I just tried this with 2.1.2rc3 but it seems to report back a version of 2.1.1. It occurred to me that buildbot also is trying to apply the permission in the configuration step, but that is failing. So putting a check here won't help that.

@@ -165,7 +165,7 @@ def _ssl_handshake(self):

return True
except ssl.SSLError as e:
raise SecurityError(e.library + ": " + e.reason)
Copy link
Contributor

Choose a reason for hiding this comment

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

library was not an attribute of SSLError in my testing. Just wrapping the exception provides more information to the user.

Copy link
Author

Choose a reason for hiding this comment

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

Which version to Python were you using? Pretty sure that is from PyOpenSSL which is only used on 2.6 and 2.7 before 2.7.10.

Copy link
Contributor

Choose a reason for hiding this comment

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

$ python2 --version
Python 2.7.10

I got some sort of exception saying that library was not an attribute of SSLError. So, I just changed it to pass the exception itself to SecurityError and I got a lot more helpful information in the output. I will reproduce and will show the difference in just a sec.

Copy link
Author

Choose a reason for hiding this comment

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

If you could try it on 2.7.9, say, that would be helpful. Then we'd know it works everywhere. Have you grepped for .library? We should change it everywhere to be consistent.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is the output with python 2.7.9 and the change to pass the exception directly to the SecurityError ctor:

  File "/home/lbakken/Projects/basho/riak-python-client/riak/transports/pbc/connection.py", line 84, in _init_security
    self._ssl_handshake()
  File "/home/lbakken/Projects/basho/riak-python-client/riak/transports/pbc/connection.py", line 168, in _ssl_handshake
    raise SecurityError(e)
SecurityError: SSLEOFError(8, u'EOF occurred in violation of protocol (_ssl.c:581)')

This is what happens with the original code:

  File "/home/lbakken/Projects/basho/riak-python-client/riak/transports/pbc/connection.py", line 84, in _init_security
    self._ssl_handshake()
  File "/home/lbakken/Projects/basho/riak-python-client/riak/transports/pbc/connection.py", line 169, in _ssl_handshake
    raise SecurityError(e.library + ": " + e.reason)
TypeError: unsupported operand type(s) for +: 'NoneType' and 'str'

Copy link
Contributor

Choose a reason for hiding this comment

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

I grepped for \.library and it didn't show up anywhere.

Copy link
Author

Choose a reason for hiding this comment

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

Note to self: Always look at the code first. Looks like 2.7.8 actually was the last version supporting PyOpenSSL. Which is interesting. I should tweak the versions in tox, then, too to reflect this.

@lukebakken
Copy link
Contributor

@javajolt I made some changes to address the cipher issue.

@lukebakken
Copy link
Contributor

I say :shipit:

@hazen
Copy link
Author

hazen commented Oct 20, 2015

+1 bdbc35f

borshop added a commit that referenced this pull request Oct 20, 2015
Add 2.1 Preflists and Write-Once Bucket Types

Reviewed-by: javajolt
@hazen
Copy link
Author

hazen commented Oct 21, 2015

@borshop merge

hazen pushed a commit that referenced this pull request Oct 21, 2015
Add 2.1 Preflists, Write-Once Bucket Types and some security enhancements
@hazen hazen merged commit e25b37c into master Oct 21, 2015
@hazen hazen deleted the feature/bch/write-once branch October 21, 2015 16:41
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