Skip to content

Added _ca_certs property to _SSLAdapter to properly support pickling (broke in SDK v11.33) #440

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

Merged
merged 2 commits into from
Aug 4, 2022

Conversation

dennissiemensma
Copy link
Contributor

@dennissiemensma dennissiemensma commented Jul 27, 2022

In Dropbox SDK v11.33 PR #385 added a new _ca_certs property to _SSLAdapter.

However, the property is dynamically specified, which breaks unpickling it using pickle.loads():

  AttributeError: '_SSLAdapter' object has no attribute '_ca_certs'

The fix is simple: Define the property instead of dynamically defining it.

Checklist

General Contributing

  • Have you read the Code of Conduct ? -> Broken! ❌
  • Have you read signed the CLA?

Is This a Code Change?

  • Non-code related change (markdown/git settings etc)
  • SDK Code Change
  • Example/Test Code Change

Validation

  • Does tox pass?
  • Do the tests pass?

@dennissiemensma
Copy link
Contributor Author

The Code of Conduct seems broken guys, HTTP 403: https://opensource.dropbox.com/coc/

@dennissiemensma
Copy link
Contributor Author

I pre-ran the tests on my fork, most succeed, but some integration tests are failing (on purpose?):

OAuth2 access token or refresh token or app key/secret must be set

@dennissiemensma dennissiemensma marked this pull request as ready for review July 27, 2022 21:13
@greg-db
Copy link
Contributor

greg-db commented Jul 28, 2022

Thanks! I'll ask the team to review this (as well as check on that 403 on the COC).

Copy link

@mover333 mover333 left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks for the new test case :)

@mover333 mover333 merged commit e1bee85 into dropbox:main Aug 4, 2022
@dennissiemensma
Copy link
Contributor Author

Great, thanks!

@greg-db
Copy link
Contributor

greg-db commented Sep 6, 2022

This is released in v11.34.0 now. Thanks again!

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.

3 participants