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 encrypt dict using property decorator #346

Conversation

benjamin-awd
Copy link

Description of the changes

Describe the changes in this PR, with references to issues in the issue tracker and forum discussions (if applicable).

Caveats

If the implementation has "sharp edges" or is otherwise incomplete, please explain here. Deviations from the contribution guidelines should also be motivated in this section.

In particular, if your contribution includes any of the following, please provide a brief justification here:

  • Changes to project dependencies.
  • Changes to existing public APIs (particularly if they could break existing user code) or the CLI.
  • Nontrivial changes to internal (non-public) APIs

Checklist

Please go over this checklist to increase the chances of your PR being worked on in a timely manner. Deviations are allowed with proper justification (see previous section).

  • I have read the project's CoC and contribution guidelines.
  • I understand and agree to the terms in the Developer Certificate of Origin as applied to this contribution.
  • All new code in this PR has full test coverage.

For new features (delete if not applicable)

  • I have discussed the implementation of this feature with the project maintainer(s) on the discussion forum or over email.
  • I have verified that my changes do not break existing API or CLI functionality, or ensured that all breaking changes are clearly documented in this PR.
  • All public API functionality in this PR is documented.

For bug fixes (delete if not applicable)

  • My changes do not affect any public API or CLI semantics.
  • My PR adds regression tests (i.e. tests that fail if the bug fix is not applied).
  • All new code in this PR has full test coverage.

For documentation contributions (delete if not applicable)

  • I have built the HTML documentation locally, and verified that the changes behave correctly in-browser.

Additional comments

Feel free to add any additional comments here.

@benjamin-awd benjamin-awd marked this pull request as draft November 23, 2023 16:18
Comment on lines +207 to +208
if self.encrypt_dict and not self._security_handler:
self._security_handler = SecurityHandler.build(self.encrypt_dict)
Copy link
Author

@benjamin-awd benjamin-awd Nov 23, 2023

Choose a reason for hiding this comment

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

Otherwise if calling self.encrypt_dict twice is an issue:

@property
def security_handler(self):
    if (encrypt_dict := self.encrypt_dict) and not self._security_handler:
        self._security_handler = SecurityHandler.build(encrypt_dict)
    return self._security_handler

@MatthiasValvekens
Copy link
Owner

MatthiasValvekens commented Nov 23, 2023

Did you see #343? :)

EDIT: oops, didn't spot that this was a diff against that branch. Was replying on mobile at the time.

@benjamin-awd
Copy link
Author

Did you see #343? :)

Ah yes I did -- just raising this as a diff, feel free to disregard this PR completely

@MatthiasValvekens MatthiasValvekens marked this pull request as ready for review November 23, 2023 19:05
@MatthiasValvekens MatthiasValvekens merged commit 9578947 into MatthiasValvekens:feature/reader-load-cleanup Nov 23, 2023
2 checks passed
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.

2 participants