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

chore: prepare 0.1.10 release #212

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

ecly
Copy link

@ecly ecly commented Aug 21, 2024

This PR prepares the 0.1.10 release. Added notes for all remaining distinct commits since 0.1.9.

Not sure 0.2.0 would be more appropriate given the breaking changes. Another option could be to change the implementation from #160 to be backwards compatible using inspect. Let me know what you think @laurentS then I'd be happy to help.

As per: #160 (comment)

@laurentS
Copy link
Owner

@ecly thank you for preparing the PR! Agree with you that we should probably bump version number to 0.2.0 to highlight the breaking change. inspect could do, if it's not running on every request, I think it's quite an expensive call, if I recall correctly.

@ecly
Copy link
Author

ecly commented Aug 22, 2024

@ecly thank you for preparing the PR! Agree with you that we should probably bump version number to 0.2.0 to highlight the breaking change. inspect could do, if it's not running on every request, I think it's quite an expensive call, if I recall correctly.

Looking at it closer, I'm actually not even sure if it should be considered backwards incompatible.
The limit.is_exempt is already checking whether the provided function accepts a request (computed with inspect on init).

The only way in which it would break anything is if a User subclasses Limit and overrides is_exempt without the request parameter, since __evaluate_limits would call it with a request paramter.

I've added code to __evaluate_limits that can handle this, but of course any subclass that doesn't implement the request parameter would essentially be breaking the contract.

So probably 3 options:

  • Ship as 0.1.10 with the changes from 8bf8b2e to make Limit subclasses without request param for is_exempt overrides still work.
  • Ship as 0.2.0 without the changes from 8bf8b2e and mention it as a breaking change for Limit class, if we consider the Limit part of the public API.
  • Ship as 0.1.10 without change from 8bf8b2e because we don't consider Limit.is_exempt part of public API.

Let me know which you prefer @laurentS then I'll prepare it.

@arielmorelli
Copy link

Any plans to release it?

@arielmorelli
Copy link

@laurentS

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