Skip to content

Conversation

stuartbeattie84
Copy link
Contributor

I have watcher code that looks something like this:

    adapter = casbin_sqlalchemy_adapter.Adapter(db_url)
    self._e = casbin.Enforcer(Config.casbin_conf_file, adapter)
    parsed_url = urllib.parse.urlparse(db_url)

    if db_adapter is True:
        self._watcher = PostgresqlWatcher(
            host=parsed_url.hostname, user=parsed_url.username, password=parsed_url.password,
            port=parsed_url.port, dbname=parsed_url.path.lstrip('/')
        )
        self._watcher.set_update_callback(self._e.load_policy)
        self._e.set_watcher(self._watcher)

        update_thread = threading.Thread(target=self.watcher_update, daemon=True)
        update_thread.start()

def watcher_update(self):
    while True:
        # should_reload() calls poll() to wait for data - returns True if there's been an update
        if self._watcher.should_reload():
            self._watcher.update_callback()

However, with the current watcher implementation the watcher_update() thread will peg CPU as the poll() call in should_reload() is non-blocking. I've changed this to a blocking call as well as removing the should_reload() call in create_subscriber_process(), which at the moment doesn't really do anything but would block the main thread if turned into a blocking call.

Unless I'm misunderstanding something, this is the only way I can get the watcher to work properly :).

@casbin-bot
Copy link

@ffyuanda @Zxilly please review

@casbin-bot casbin-bot requested a review from ffyuanda May 5, 2022 08:01
@CLAassistant
Copy link

CLAassistant commented May 5, 2022

CLA assistant check
All committers have signed the CLA.

@hsluoyz
Copy link
Member

hsluoyz commented May 5, 2022

@leeqvip

@hsluoyz
Copy link
Member

hsluoyz commented May 10, 2022

@stuartbeattie84 plz fix:

image

@stuartbeattie84
Copy link
Contributor Author

Sorted licensing issue :)

@gmc77
Copy link

gmc77 commented May 13, 2022

Hi, this is currently blocking us. Is there an ETA for when we can get his merged into a release? Thanks!

@hsluoyz
Copy link
Member

hsluoyz commented May 13, 2022

@tangyang9464 @abichinger @Abingcbc plz review

@hsluoyz
Copy link
Member

hsluoyz commented May 13, 2022

@gmc77 plz review

@hsluoyz hsluoyz changed the title Made should_reload into a blocking call fix: made should_reload into a blocking call May 17, 2022
@hsluoyz hsluoyz merged commit 6fd89b5 into pycasbin:master May 17, 2022
github-actions bot pushed a commit that referenced this pull request May 17, 2022
## [0.1.2](v0.1.1...v0.1.2) (2022-05-17)

### Bug Fixes

* made should_reload into a blocking call ([#21](#21)) ([6fd89b5](6fd89b5))
@github-actions
Copy link

🎉 This PR is included in version 0.1.2 🎉

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.

5 participants