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

Discarding origin is brittle #234

Open
fcollonval opened this issue Feb 17, 2025 · 2 comments · May be fixed by #233
Open

Discarding origin is brittle #234

fcollonval opened this issue Feb 17, 2025 · 2 comments · May be fixed by #233
Labels
bug Something isn't working

Comments

@fcollonval
Copy link
Member

I find the cleaning of the origin too brittle:

del self._doc._origins[origin_hash]

Reasons:

  • You cannot be sure the transaction will be discarded at exit.
  • The hashed_origin obtained is not the one stored in the __init__, how could you be sure you are not removing a reference to another hash?
  • The resource clearing is not happening in the mirror action of its creation; aka init <=> del
@davidbrochart
Copy link
Collaborator

  • The hashed_origin obtained is not the one stored in the __init__, how could you be sure you are not removing a reference to another hash?

You mean the hashed origin obtained in __exit__? If so, yes it is the one stored in __enter__, previously computed and registered in __init__. Am I missing something?

  • The resource clearing is not happening in the mirror action of its creation; aka init <=> del

A transaction should only be used with a context manager, so registering an origin in __init__ is equivalent to registering it in __enter__, which is symmetrical to the clearing happening in __exit__.
But I agree that users could create the Transaction object and never use it in a context manager, for whatever reason. Moving the registering to __enter__ should handle that case, right? I guess that's also what you meant by:

  • You cannot be sure the transaction will be discarded at exit.

@fcollonval fcollonval added the bug Something isn't working label Feb 18, 2025
@fcollonval fcollonval linked a pull request Feb 19, 2025 that will close this issue
@fcollonval
Copy link
Member Author

In adding the transaction to event - I needed to revisit that part. See #233 for a proposal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants