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.

Sign up for GitHub

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

Jump to bottom

Fix FileWatcher ordering bug #51

Closed
shsms opened this issue Nov 18, 2022 · 3 comments
Closed

Fix FileWatcher ordering bug #51

shsms opened this issue Nov 18, 2022 · 3 comments
Assignees
@sahas-subramanian-frequenz
Labels
part:core Affects the core types (`Sender`, `Receiver`, exceptions, etc.) resolution:invalid This doesn't seem right type:bug Something isn't working

Comments

@shsms
Copy link
Contributor

shsms commented Nov 18, 2022 •
edited

It looks like ordering is important here and I don't think set preserves order. Also, why do we want to compress FileChanges that are the same, suppose we have this sequence:

  1. /a is added
  2. /a is deleted
  3. /a is added again

With this implementation we could only get one add and one deletion so we would think the file doesn't exist but it does.

If this introduced a bug that wasn't detected by test, we should probably add a test for this too :)

Originally posted by @leandro-lucarella-frequenz in #42 (comment)
@shsms

This comment was marked as off-topic.

Sign in to view
@shsms shsms mentioned this issue Nov 18, 2022
Merged
@sahas-subramanian-frequenz sahas-subramanian-frequenz added type:bug Something isn't working part:core Affects the core types (`Sender`, `Receiver`, exceptions, etc.) labels Nov 21, 2022
@leandro-lucarella-frequenz leandro-lucarella-frequenz changed the title fix filewatcher ordering bug Fix FileWatcher ordering bug Nov 21, 2022
Closed
@leandro-lucarella-frequenz

This comment was marked as off-topic.

Sign in to view
@leandro-lucarella-frequenz leandro-lucarella-frequenz added the resolution:invalid This doesn't seem right label Nov 21, 2022
@leandro-lucarella-frequenz
Copy link
Contributor

Looking more closely at watchfiles documentation, it looks like the library doens't really provide ordering anyway, as it yields a Set[FileChange].

So closing as invalid.

@leandro-lucarella-frequenz leandro-lucarella-frequenz closed this as not planned Won't fix, can't repro, duplicate, stale Nov 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sahas-subramanian-frequenz sahas-subramanian-frequenz

Labels
part:core Affects the core types (`Sender`, `Receiver`, exceptions, etc.) resolution:invalid This doesn't seem right type:bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@shsms @sahas-subramanian-frequenz @leandro-lucarella-frequenz