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

make qtsass watchdog dependence optional #41

Closed
2 tasks
ewerybody opened this issue Sep 2, 2019 · 3 comments
Closed
2 tasks

make qtsass watchdog dependence optional #41

ewerybody opened this issue Sep 2, 2019 · 3 comments
Assignees
Milestone

Comments

@ewerybody
Copy link
Contributor

Heya! Just for clearing up all the things discussed:

Agreed, we can make it optional then, make sure to update the setup.py

What needs to happen there? You mean just like

That aint all right?

@goanpeca goanpeca added this to the 0.2.0 milestone Sep 4, 2019
@goanpeca goanpeca self-assigned this Sep 4, 2019
@danbradham
Copy link
Collaborator

After thinking about this a little bit I think I'd like to do the following:

  1. Create a watchers subpackage containing a Watcher baseclass.
  2. Create a watchdog implementation of the Watcher API.
  3. Import a Watcher implementation in the api.watch function just prior to use.

This would concentrate the point of failure in a single location making it easier to manage. It would also allow us to create fallback implementations.

A QT Watcher would probably make more sense than watchdog as we can be somewhat sure that QT would be available in a project using qtsass. There's one issue here in that the QFileSystemWatcher is actually pretty incomplete. It doesn't handle recursive directory watching and requires a QApplication event loop to work at all if I remember correctly.

We could also write a simple polling Watcher that polls the filesystem in a thread checking modified times of files and folders. This is roughly equivalent to watchdogs fallback Observer. If we went this route we could eliminate watchdog as a dependency entirely.

I think I have some bandwidth to put together a wip PR for this tomorrow. At the bare minimum tackling the first 3 steps I listed above.

@ccordoba12
Copy link
Member

If watchdog is used for development only, why not adding it to extras_require instead of reinventing the wheel?

@danbradham danbradham mentioned this issue Sep 4, 2019
3 tasks
@goanpeca
Copy link
Member

goanpeca commented Sep 4, 2019

Agreed with @ccordoba12 here @danbradham

Th fix for this was to add an extras_require on setup.py. Or add the try except on imports since this is not really a crucial functionality

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

No branches or pull requests

4 participants