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

Allow pueue status to order tasks by enqueue_at #554

Closed
pacien opened this issue Jul 20, 2024 · 4 comments · Fixed by #559
Closed

Allow pueue status to order tasks by enqueue_at #554

pacien opened this issue Jul 20, 2024 · 4 comments · Fixed by #559
Labels
f: Help wanted s: Client This issue touches the pueue client t: Bug

Comments

@pacien
Copy link
Contributor

pacien commented Jul 20, 2024

pueue status order_by enqueue_at [asc/desc]
does not appear to order tasks as requested.

Minimal non-working example:

❯ pueue add --delay 2024-07-21 echo hi
New task added (id 0). It will be enqueued at 2024-07-21 00:00:00

❯ pueue add --delay 2024-07-22 echo hi
New task added (id 1). It will be enqueued at 2024-07-22 00:00:00

❯ pueue add --delay 2024-07-21 echo hi
New task added (id 2). It will be enqueued at 2024-07-21 00:00:00

❯ pueue status order_by enqueue_at
Group "default" (1 parallel): running
───────────────────────────────────────────────────────────────
 Id   Status    Enqueue At   Command   Path        Start   End 
═══════════════════════════════════════════════════════════════
 2    Stashed   00:00:00     echo hi   /home/user               
───────────────────────────────────────────────────────────────
 1    Stashed   2024-07-22   echo hi   /home/user               
                00:00:00                                       
───────────────────────────────────────────────────────────────
 0    Stashed   00:00:00     echo hi   /home/user               
───────────────────────────────────────────────────────────────

❯ pueue status order_by enqueue_at desc
Group "default" (1 parallel): running
───────────────────────────────────────────────────────────────
 Id   Status    Enqueue At   Command   Path        Start   End 
═══════════════════════════════════════════════════════════════
 0    Stashed   00:00:00     echo hi   /home/user               
───────────────────────────────────────────────────────────────
 1    Stashed   2024-07-22   echo hi   /home/user               
                00:00:00                                       
───────────────────────────────────────────────────────────────
 2    Stashed   00:00:00     echo hi   /home/user               
───────────────────────────────────────────────────────────────

Expected output:
task ID 1 shouldn't be in-between in either cases.

Platform info:

  • pueue 3.4.0
  • NixOS 24.05 (GNU/Linux 6.6.32, x86_64)
@Nukesor
Copy link
Owner

Nukesor commented Jul 21, 2024

Feel free to implement this :) Please add proper tests as well.

Also: Please don't mark missing features as bugs.
A bug is something that doesn't work as intended. If you're missing some functionality that doesn't exist yet, it's a feature request.

@Nukesor Nukesor changed the title [Bug] CLI / status: order_by enqueue_at not working [Feature request] CLI / status: order_by enqueue_at not working Jul 22, 2024
@Nukesor Nukesor added f: Help wanted t: Feature A new feature that needs implementation s: Client This issue touches the pueue client labels Jul 22, 2024
@Nukesor Nukesor changed the title [Feature request] CLI / status: order_by enqueue_at not working CLI / status: order_by enqueue_at not working Jul 22, 2024
@Nukesor Nukesor changed the title CLI / status: order_by enqueue_at not working Allow pueue status to order tasks by enqueue_at Jul 22, 2024
@pacien
Copy link
Contributor Author

pacien commented Jul 23, 2024

Hi!

Feel free to implement this :) Please add proper tests as well.

I'll try to submit some changeset for this.

Also: Please don't mark missing features as bugs. A bug is something that
doesn't work as intended. If you're missing some functionality that doesn't
exist yet, it's a feature request.

The help text of the pueue status sub-command indicates that enqueue_at
is a possible order to give to the order_by clause.

The query is indeed accepted without any error or warning,
producing some unexpected effect, even more so with asc/desc.
So that looks like a bug from my point of view of user.

Perhaps the grammar could be tightened, so that the order_by clause only
accepts a subset of columns, similarly to what's being done for the filter
clause with filter_column?

@Nukesor Nukesor added t: Bug and removed t: Feature A new feature that needs implementation labels Jul 24, 2024
@Nukesor
Copy link
Owner

Nukesor commented Jul 24, 2024

You're right, I completely forgot about those docs as I didn't write them myself. Sorry for that.

Yep, according to the docs, enqueue_at should be possible to be ordered by. Should be fairly straight forward to add it to https://github.com/Nukesor/pueue/blob/main/pueue/src/client/query/mod.rs#L65

I would wait for #556 to be merged first though, as this changes quite a lot of stuff.

@Nukesor
Copy link
Owner

Nukesor commented Jul 26, 2024

#556 is merged :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
f: Help wanted s: Client This issue touches the pueue client t: Bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants