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

support a [pserve] config section with a list of files to watch #2827

Merged
merged 5 commits into from Dec 10, 2016

Conversation

Projects
None yet
3 participants
@mmerickel
Copy link
Member

commented Nov 21, 2016

fixes #2732

@mmerickel mmerickel force-pushed the mmerickel:pserve-watch-files branch from bbb686b to 361adfc Nov 21, 2016

@mmerickel mmerickel force-pushed the mmerickel:pserve-watch-files branch from 361adfc to 3f13093 Nov 21, 2016

@mmerickel

This comment has been minimized.

Copy link
Member Author

commented Nov 21, 2016

I'm thinking that I will update this to support asset specs such that if there's a : in the name it will resolve it. For example,

[pserve]
watch_files =
    myapp:static/foo.png         # relative to myapp package
    extra_files/bar.png          # relative to ini file ??? cwd?
    %(here)s/extra_files/baz.png # relative to ini file

Currently everything is relative to the ini file and the %(here)s syntax is not supported. Does anyone have some thoughts on whether I should support the %(here)s syntax?

I think ideally extra_files/bar.png should be relative to the app but it's not really possible because this is loaded by pserve, not by the app, so we don't really know where the app's package is or what it is. The only sane thing seems to be relative to the ini file.

@ztane

This comment has been minimized.

Copy link
Contributor

commented Nov 21, 2016

Well, I wouldn't support cwd-relativeness at all, usually it is just PITA because it is difficult to tell what the cwd happens to be at which time. I'd support full variable interpolations though, not only here.

And how about globbing. I wouldn't want to change my development.ini all the time. Just pass the final "filename" into glob.glob() at initialization time (let's not consider new files yet), to get a list of resulting paths. (And too bad that recursive globbing doesn't exist < 3.5)

@ztane

This comment has been minimized.

Copy link
Contributor

commented Nov 21, 2016

hmm though... glob.glob('foo.bar') where foo.bar doesn't exist returns empty list; so if it should watch new named files that don't exist yet...

@mmerickel

This comment has been minimized.

Copy link
Member Author

commented Nov 21, 2016

Globbing is coming, I just need to think about it a bit more. It will be handled by hupper and so it won't require any changes to this PR aside from a doc improvement.

Originally I wanted to support a glob that would actually watch for new files that matched... but it's somewhat sucky to implement so I might just re-evaluate the glob after each restart.

@ztane

This comment has been minimized.

Copy link
Contributor

commented Nov 21, 2016

well, if it is possible to register a new matcher, then someone could write a version that uses inotify say

@mmerickel

This comment has been minimized.

Copy link
Member Author

commented Nov 21, 2016

well, if it is possible to register a new matcher, then someone could write a version that uses inotify say

me thinks you haven't read anything about hupper

@mmerickel mmerickel added this to the 1.8 milestone Nov 21, 2016

@ztane

This comment has been minimized.

Copy link
Contributor

commented Nov 21, 2016

Obviously no :D so perhaps just as simple as rewrite glob patterns into a regex for WatchdogFileMonitor à la fnmatch.translate (but better), trivial to support ** for recursive-glob on Python <3.5 too. Don't know how the polling should work, though

@mmerickel mmerickel force-pushed the mmerickel:pserve-watch-files branch from b6220e1 to d1d66cf Nov 22, 2016

@mmerickel mmerickel force-pushed the mmerickel:pserve-watch-files branch from d1d66cf to 9cab0e7 Nov 22, 2016

@mmerickel

This comment has been minimized.

Copy link
Member Author

commented Nov 22, 2016

Okay I added support for some variable interpolation as well as asset specs. I'm looking at adding at least some basic glob support to hupper now.

@mmerickel

This comment has been minimized.

Copy link
Member Author

commented Nov 22, 2016

This is ready for review, I added some super basic globbing support to hupper by passing the patterns directly into glob.glob.

@bertjwregeer

This comment has been minimized.

Copy link
Member

commented Dec 10, 2016

Code looks good. Testing.

@bertjwregeer

This comment has been minimized.

Copy link
Member

commented Dec 10, 2016

LGTM and works!

@bertjwregeer bertjwregeer merged commit 98b7bc9 into Pylons:master Dec 10, 2016

0 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.