Skip to content

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

Merged
mover333 merged 2 commits into
dropbox:mainfrom
dennissiemensma:bug/_ca_certs-pickle
Aug 4, 2022
Merged

Added _ca_certs property to _SSLAdapter to properly support pickling (broke in SDK v11.33)#440
mover333 merged 2 commits into
dropbox:mainfrom
dennissiemensma:bug/_ca_certs-pickle

Conversation

@dennissiemensma

@dennissiemensma dennissiemensma commented Jul 27, 2022

Copy link
Copy Markdown
Contributor

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
Copy Markdown
Contributor Author

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

@dennissiemensma

Copy link
Copy Markdown
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

greg-db commented Jul 28, 2022

Copy link
Copy Markdown
Contributor

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

@mover333 mover333 left a comment

Copy link
Copy Markdown

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
Copy Markdown
Contributor Author

Great, thanks!

@greg-db

greg-db commented Sep 6, 2022

Copy link
Copy Markdown
Contributor

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