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

Never read with cockpit.file('/boot/bigfile').watch(callback, {'read': false }) #19983

Closed
jelly opened this issue Feb 9, 2024 · 3 comments
Closed

Comments

@jelly
Copy link
Member

jelly commented Feb 9, 2024

cockpit.file('/boot/bigfile').watch(callback, {'read': false })

This would initially when setting up read the /boot/bigfile which is undesired behaviour caused by this. Not calling read() when options.read == false seems to hang watch() so more work is required to get this fixed.

Unit test for verification: a6838ab

@jelly jelly added the bug label Feb 9, 2024
@jelly jelly changed the title RFC: never read with cockpit.file('/boot/bigfile').watch(callback, {'read': false }) Never read with cockpit.file('/boot/bigfile').watch(callback, {'read': false }) Feb 9, 2024
@martinpitt
Copy link
Member

The docs does mention the initial read(), but indeed we need to fix that to not happen with read: false. Also, the bit of "This might happen because of external changes, but also as part of calls to read(), [...]" sounds utterly wrong: reading a file should not trigger the watch handler. If it does, then I suppose it's established API and a weird quirk, but then we should create an unit test and make it definite ("might" doesn't make this predictable at all).

However, it does make sense to always call the watch handler initially, so that consumers don't need to call it explicitly once - you almost always want this for initialization. But we should describe it as such.

That test does not modify the file after setting up the watch, so why would your handler ever be called? This may be related to .read() forcing a watch handler callback, or just badly worded. So indeed this needs to be fixed -- we need to replace that .read() with calling the watch handler on init with read: false, fix up that utterly confusing documentation, and pin down the behaviour in unit tests.

jelly added a commit to jelly/cockpit that referenced this issue Feb 13, 2024
By using the new fsinfo channel fswatch1 can now read the tag without
reading the file when adding a new watch. As side-effect this no longer
reads the full file when `{ read: false }` is passed to `watch()`.

Closes: cockpit-project#19983
jelly added a commit to jelly/cockpit that referenced this issue Feb 14, 2024
By using the new fsinfo channel fswatch1 can now read the tag without
reading the file when adding a new watch. As side-effect this no longer
reads the full file when `{ read: false }` is passed to `watch()`.

Closes: cockpit-project#19983
jelly added a commit to jelly/cockpit that referenced this issue Feb 19, 2024
By using the new fsinfo channel fswatch1 can now read the tag without
reading the file when adding a new watch. As side-effect this no longer
reads the full file when `{ read: false }` is passed to `watch()`.

Closes: cockpit-project#19983

Co-Authored-By: Allison Karlitskaya <allison.karlitskaya@redhat.com>
@VivekJaiswal18
Copy link

@jelly @martinpitt Is Cockpit-Project taking part in GSoC 2024?

@jelly
Copy link
Member Author

jelly commented Feb 19, 2024

@VivekJaiswal18 so far we have no GSoC 2024 plans, please don't use issues to ask about GSoC and use Github's discussions feature.

jelly added a commit to jelly/cockpit that referenced this issue Feb 19, 2024
By using the new fsinfo channel fswatch1 can now read the tag without
reading the file when adding a new watch. As side-effect this no longer
reads the full file when `{ read: false }` is passed to `watch()`.

Closes: cockpit-project#19983

Co-Authored-By: Allison Karlitskaya <allison.karlitskaya@redhat.com>
jelly added a commit to jelly/cockpit that referenced this issue Feb 20, 2024
By using the new fsinfo channel fswatch1 can now read the tag without
reading the file when adding a new watch. As side-effect this no longer
reads the full file when `{ read: false }` is passed to `watch()`.

Closes: cockpit-project#19983

Co-Authored-By: Allison Karlitskaya <allison.karlitskaya@redhat.com>
jelly added a commit to jelly/cockpit that referenced this issue Feb 21, 2024
By using the new fsinfo channel fswatch1 can now read the tag without
reading the file when adding a new watch. As side-effect this no longer
reads the full file when `{ read: false }` is passed to `watch()`.

Closes: cockpit-project#19983

Co-Authored-By: Allison Karlitskaya <allison.karlitskaya@redhat.com>
jelly added a commit to jelly/cockpit that referenced this issue Mar 14, 2024
By using the new fsinfo channel fswatch1 can now read the tag without
reading the file when adding a new watch. As side-effect this no longer
reads the full file when `{ read: false }` is passed to `watch()`.

Closes: cockpit-project#19983

Co-Authored-By: Allison Karlitskaya <allison.karlitskaya@redhat.com>
jelly added a commit to jelly/cockpit that referenced this issue Mar 21, 2024
By using the new fsinfo channel fswatch1 can now read the tag without
reading the file when adding a new watch. As side-effect this no longer
reads the full file when `{ read: false }` is passed to `watch()`.

Closes: cockpit-project#19983

Co-Authored-By: Allison Karlitskaya <allison.karlitskaya@redhat.com>
jelly added a commit to jelly/cockpit that referenced this issue Mar 22, 2024
By using the new fsinfo channel fswatch1 can now read the tag without
reading the file when adding a new watch. As side-effect this no longer
reads the full file when `{ read: false }` is passed to `watch()`.

Closes: cockpit-project#19983

Co-Authored-By: Allison Karlitskaya <allison.karlitskaya@redhat.com>
jelly added a commit to jelly/cockpit that referenced this issue Apr 4, 2024
By using the new fsinfo channel fswatch1 can now read the tag without
reading the file when adding a new watch. As side-effect this no longer
reads the full file when `{ read: false }` is passed to `watch()`.

Closes: cockpit-project#19983

Co-Authored-By: Allison Karlitskaya <allison.karlitskaya@redhat.com>
jelly added a commit to jelly/cockpit that referenced this issue Jun 25, 2024
By using the new fsinfo channel fswatch1 can now read the tag without
reading the file when adding a new watch. As side-effect this no longer
reads the full file when `{ read: false }` is passed to `watch()`.

Closes: cockpit-project#19983

Co-Authored-By: Allison Karlitskaya <allison.karlitskaya@redhat.com>
jelly added a commit to jelly/cockpit that referenced this issue Jun 25, 2024
By using the new fsinfo channel fswatch1 can now read the tag without
reading the file when adding a new watch. As side-effect this no longer
reads the full file when `{ read: false }` is passed to `watch()`.

Closes: cockpit-project#19983

Co-Authored-By: Allison Karlitskaya <allison.karlitskaya@redhat.com>
allisonkarlitskaya added a commit to jelly/cockpit that referenced this issue Jun 27, 2024
By using the new fsinfo channel fswatch1 can now read the tag without
reading the file when adding a new watch. As side-effect this no longer
reads the full file when `{ read: false }` is passed to `watch()`.

Closes: cockpit-project#19983

Co-Authored-By: Allison Karlitskaya <allison.karlitskaya@redhat.com>
jelly added a commit to jelly/cockpit that referenced this issue Jun 27, 2024
By using the new fsinfo channel fswatch1 can now read the tag without
reading the file when adding a new watch. As side-effect this no longer
reads the full file when `{ read: false }` is passed to `watch()`.

Closes: cockpit-project#19983

Co-Authored-By: Allison Karlitskaya <allison.karlitskaya@redhat.com>
jelly added a commit to jelly/cockpit that referenced this issue Jun 27, 2024
By using the new fsinfo channel fswatch1 can now read the tag without
reading the file when adding a new watch. As side-effect this no longer
reads the full file when `{ read: false }` is passed to `watch()`.

Closes: cockpit-project#19983

Co-Authored-By: Allison Karlitskaya <allison.karlitskaya@redhat.com>
jelly added a commit to jelly/cockpit that referenced this issue Jun 27, 2024
By using the new fsinfo channel fswatch1 can now read the tag without
reading the file when adding a new watch. As side-effect this no longer
reads the full file when `{ read: false }` is passed to `watch()`.

Closes: cockpit-project#19983

Co-Authored-By: Allison Karlitskaya <allison.karlitskaya@redhat.com>
jelly added a commit to jelly/cockpit that referenced this issue Jun 27, 2024
By using the new fsinfo channel fswatch1 can now read the tag without
reading the file when adding a new watch. As side-effect this no longer
reads the full file when `{ read: false }` is passed to `watch()`.

Closes: cockpit-project#19983

Co-Authored-By: Allison Karlitskaya <allison.karlitskaya@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants