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

feat: Added support for async watcher callbacks #340 #341

Merged

Conversation

tanasecucliciu
Copy link
Contributor

@tanasecucliciu tanasecucliciu commented Jan 29, 2024

AsyncEnforcer can now also await Watcher callbacks if they are Coroutines. Related to #340.

Fix: #340

AsyncEnforcer can now also await Watcher callbacks if they are Coroutines.
@casbin-bot
Copy link
Member

@Nekotoxin please review

@CLAassistant
Copy link

CLAassistant commented Jan 29, 2024

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@leeqvip leeqvip left a comment

Choose a reason for hiding this comment

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

Can you add some unit tests to them?

@tanasecucliciu
Copy link
Contributor Author

Sure thing!

I've added pytests to verify the functionality of async watcher callbacks.

Note: I used the existing pytest for synchronous calls as a reference for creating this async version.
@hsluoyz
Copy link
Member

hsluoyz commented Feb 1, 2024

@tanasecucliciu fix linter

image

@tanasecucliciu tanasecucliciu force-pushed the feat/340-async-enforcer-async-update-for branch from 16c2489 to 6657f12 Compare February 1, 2024 20:15

await e.save_policy()
self.assertEqual(w.notify_message, "called save policy")

Copy link
Member

Choose a reason for hiding this comment

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

Whether save_policy should also depend on auto_notify_watcher ?

Copy link
Member

Choose a reason for hiding this comment

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

@hsluoyz I saw the same in go's casbin.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any updates on this issue?

Copy link
Member

Choose a reason for hiding this comment

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

@leeqvip it should behave the same way as Go

@leeqvip leeqvip merged commit c04d832 into casbin:master Feb 9, 2024
30 checks passed
github-actions bot pushed a commit that referenced this pull request Feb 9, 2024
# [1.36.0](v1.35.0...v1.36.0) (2024-02-09)

### Features

* Added support for async watcher callbacks [#340](#340) ([#341](#341)) ([c04d832](c04d832))
Copy link

github-actions bot commented Feb 9, 2024

🎉 This PR is included in version 1.36.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AsyncEnforcer doesn't support async calls for Watcher's update function
5 participants