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

WIP: Split OpenSSL binding from the backend interface #380

Merged
merged 1 commit into from Jan 2, 2014

Conversation

public
Copy link
Member

@public public commented Dec 31, 2013

No description provided.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 10e6efd on public:split-binding into fbd7ffc on pyca:master.

@jenkins-cryptography
Copy link

Test FAILed.
Refer to this link for build results: http://jenkins.cryptography.io/job/cryptography-pr-experimental/127/

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 1d56c5e on public:split-binding into fbd7ffc on pyca:master.

@jenkins-cryptography
Copy link

Test PASSed.
Refer to this link for build results: http://jenkins.cryptography.io/job/cryptography-pr-experimental/128/

@alex
Copy link
Member

alex commented Dec 31, 2013

Two notes:

  1. I don't think subclassing is the right approach here.
  2. bindings can't live in the backends/openssl/ dir, this is because openssl/init imoprts openssl/backend which instantiates a backend and does all the initialization logic.

@public
Copy link
Member Author

public commented Dec 31, 2013

Bah missed that module scope Backend() call. Why not subclassing? You'd rather the backend just pulled the lib and ffi off of another object?

@jenkins-cryptography
Copy link

Test PASSed.
Refer to this link for build results: http://jenkins.cryptography.io/job/cryptography-pr-experimental/133/

@jenkins-cryptography
Copy link

Test PASSed.
Refer to this link for build results: http://jenkins.cryptography.io/job/cryptography-pr-experimental/134/


class TestOpenSSL(object):
def test_binding_loads(self):
assert Binding()
Copy link
Contributor

Choose a reason for hiding this comment

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

This should probably assert that both lib and ffi exist and are not None.

@jenkins-cryptography
Copy link

Test PASSed.
Refer to this link for build results: http://jenkins.cryptography.io/job/cryptography-pr-experimental/140/

@alex
Copy link
Member

alex commented Jan 1, 2014

This looks good, except there's a merge conflict, can you merge/rebase master in?

@jenkins-cryptography
Copy link

Test PASSed.
Refer to this link for build results: http://jenkins.cryptography.io/job/cryptography-pr-experimental/147/

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 82c8cc1 on public:split-binding into c9add40 on pyca:master.

@@ -67,6 +67,7 @@ The hazardous materials layer

hazmat/primitives/index
hazmat/backends/index
hazmat/bindings/index
Copy link
Member

Choose a reason for hiding this comment

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

It looks like this file wasn't added to the index.

@alex
Copy link
Member

alex commented Jan 1, 2014

And it looks like there's somehow still a merge conflict issue?

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling c368ac2 on public:split-binding into 5f4c492 on pyca:master.

@jenkins-cryptography
Copy link

Test PASSed.
Refer to this link for build results: http://jenkins.cryptography.io/job/cryptography-pr-experimental/154/

alex added a commit that referenced this pull request Jan 2, 2014
WIP: Split OpenSSL binding from the backend interface
@alex alex merged commit 9221736 into pyca:master Jan 2, 2014
joerichter-stash pushed a commit to kiwigrid/cryptography that referenced this pull request Nov 15, 2017
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants