Folder monitor #72
Open
Folder monitor #72
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
ajCameron
commented
Aug 3, 2022
ajCameron
commented
- allows monitoring of a file - or folder - for changes which generate input events
- allows files to be written out to a destination folder as the result of an output event
- provides a prototype for the sort of comprehensive tests it would be good to have for all the IO methods
- and an example of the expanded dev notes, for comment.
- might not even be broken anymore!
- Have a working prototype which awaits file system events and then produces appropriate InputEvent classes in response - User passes in a path to monitor - Currently dealing with the user passing in the path None by rechecking using async.sleep to see if the property has been updated - but this is less than elegant - And needs testing - Added notes as to the reason for the choice of library and some on design - these may be cut before the final PR - requiremnets.txt updated with the two new files being used to enable this functionality - watchfiles and aiopath - examples added for watching - an existing single file - a file path which is, in fact, None
- Added pytest-asyncio to handle the expanded test requirements - A mock of the FileSystemInput is now being run in an event loop - Introduced a folder structure to the tests to reflect the project folder structue - All tests current passing - hence this commit, while this is still true.
- Hoping to bypass the complexity of dealing with proper folder monitoring. Not sure if it will work - hence this commit
- There are now two input classes rather than one - one for monitoring a dir, and one for monitoring a folder - added fallbacks and monitoring to make sure the watchers don't silently die when the target file is deleted - updated examples to reflect you now have to specify the type of resource you want to monitor - tests to be re-instituted - still dealing with a bug where running the test suite does not release control to the terminal after the test is complete
- Main file was edging up towards a thousand lines - Currently broken down into, "events", "input" and "output"
- Previously, the type of event produced when the target object was created or deleted was just a standard file or dir event - the user would have to test each event themselves to see if it was a creation or deletion event which removed the monitored file - Now a specific event is used, to signal to the user that monitoring is starting or stopping - dev print statements removed
WINDOWS 10 - PYTHON 3.10 - During testing, noted that pytest was not exiting cleanly after test run - was not yielding back to the terminal - key board interrupt did nothing. Like the goggles. - isolated the problem to the presence or absence of a async queue in the test case - del queue has not worked - overwriting the queue with None in the running task has not worked - calling task_done on the queue does not work - queue appears to support no good methods for manually triggering shutdown Next step - mock the queue with a wrapped list - if no better solution can be found.
- Clean test problem solved by capturing and cancelling the input task - which destroys the queue - Additional serious issues emerged when testing the folder monitor on windows. These included, but where not limited to - - generating update events after a file or folder was deleted - - system being fooled into thinking a file was never deleted by replacing it really fast - - move events generating an update first - - directory delete events generating file delete events - a number of these seem to be limitations of the underlying API. But some are just inexplicable - These where patched with a number of hacks and some data caching - This is far from an ideal solution - the monitor components have been spun off into separate classes - so they can be replaced on an OS by OS basis (suspect the basis will be "Windows" and "Not Windows" - though Darwin and Haiku may have some surprises - testing has been streamlined and fixtures introduced - including a utils for common testing tasks - additional notes on this problem in file_system-dev-notes.md
- basic four events have been implemented for output - tests for the four events are included
- does not produce the same result as the windows smoothing - so might have to adjust so the end results are the same
…lts) - This is not going well. - Either a massive amount of complexity needs to be added, or the user is just going to have to accept that windows and linux file monitors behave slightly differently. - As bad an option as it is, I think the increased complexity is just not worth it.
- will return to Windows dev to continue producing smoothing - standardizing on what the linux ionide monitor produced.
- Now to check a few more Linux side things - and to make sure all the linting passes
- Some patches have been required - due to inconsistent results from the underlying monitor - The reasons for this are not fully explored
- Some patches have been required - due to inconsistent results from the underlying monitor - The reasons for this are not fully explored
- different tests had to be written for windows and linux like systems - tests are set to autoskip if they are run on the wrong type of host os - additional smoothing was undertaken - at least a minimal useful set of events are being produced now from each underlying event - additional explanatory notes added
- This time running on the remote testing backend on github
- Refactoring to use generic methods where possible - As preparation to introducing timeouts for the async tests
- Now tested and working on linux - but with an intermittent bug in the tests which is deeply concerning
- Codebase DRYed out - Mostly hoping for some more info from those silently failing tests
- To more closely confirm with sonarcloud guidelines
- DRYing code - in particular the tests - addressing code smells - when this can be done without conflicting another of the linters
- Additional drying of the code base - further removal of code smells.
- Further effort to remove code smells
javajawa
force-pushed
the
folder_monitor
branch
from
ad88f07 to
aa18b10
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
|
|
||
| async def act(self, event: InputEvent, state: Dict[str, Any]) -> None: | ||
| """ | ||
| Construct a DiscordOutputEvent with the result of performing the calculation. |
There was a problem hiding this comment.
Suggested change
| Construct a DiscordOutputEvent with the result of performing the calculation. | |
| Print a logging message |
|
|
||
| async def act(self, event: InputEvent, state: Dict[str, Any]) -> None: | ||
| """ | ||
| Construct a DiscordOutputEvent with the result of performing the calculation. |
There was a problem hiding this comment.
Suggested change
| Construct a DiscordOutputEvent with the result of performing the calculation. | |
| Print a logging message of the event is appropriate. |
|
SonarCloud Quality Gate failed. |
|
SonarCloud Quality Gate failed.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.