FR: track task creation time about pueue HOT 15 CLOSED

ping @dmgolembiowski

Do you still plan to work on this :)? I intend to do a release in the upcoming weeks and this is something I would like to see in that release^^. In case you're no longer interested, I'll probably continue working on this :)

from pueue.

I don't see any problems with this :)

If we track the created_at time, I would even go ahead and track the enqueued_at time as well.
The enqueue_at time is currently only saved in the task's state and not persistant. Once the task is automatically enqueued, this information is lost.

• created_at: Datetime should be fairly straight forward. Just set the field in the Task constructor.
• enqueued_at: Option<Datetime> however has a few places where it needs to be set (all inside the daemon)
  • When handling a task enqueue
  • When adding a task without --stashed
  • When restarting a task
  • When stashing a task (reset it to None)
  • Probably somewhere I'm currently not thinking of

from pueue.

Thanks for taking care of this issue :)

That test is rather important as it ensures backward compatibility for states of old versions.
If we introduce incompatibilities, users will loose their current state when restarting the daemon after updating.

The proper way around it would be to define default deserialization values for the missing fields.
Check out serde defaults.

That way, old states might have incorrect values, but at least they can be parsed and used as expected.

from pueue.

Thanks for taking care of this issue :)
You're welcome; I see a ton of potential for pueue.

The proper way around it would be to define default deserialization values for the missing fields. Check out serde defaults.

That way, old states might have incorrect values, but at least they can be parsed and used as expected.

Sweet, I'll go ahead and work that out later tonight

from pueue.

@dmgolembiowski I added a few comments directly on your commits, since the repository referenced in the PR no longer exists.

Regarding tests, I would be ok with adding a few statements in the tests that check enqueing and adding tasks :)

I.e.

• test_normal_add for created_at.
• test_enqueued_tasks and test_delayed_tasks for enqueue_at

Since we cannot check for exact times, we'll have to make do with a appropriate time range (~500ms-1000ms)

from pueue.

Should editing a task's command/path update the created_at field?

Nope. It’s the same task still as far as I am concerned. If I need it to have a new created time I’ll create a new task…

from pueue.

Ping @dmgolembiowski

How is it going :)? In case you are no longer working on this, I'll continue to work on this :)
This should definitely be in the next release. I'm going on holiday next week and I'll try to polish everything for a 3.0 release during that time.

from pueue.

Implemented in #386

from pueue.

@mjpieters + @Nukesor, I'll give this a shot tonight.

@Nukesor I suspect you were right that there was somewhere else you weren't thinking of originally. After implementing the tweaks in a few spots, running cargo test shows an error restoring from an old state.

File "/home/david/pueue/tests/state_backward_compatibility.rs", line 19, in state_backward_compatibility::test_restore_from_old_state::{{closure}}
    fn test_restore_from_old_state() -> Result<()> { 

which has:

        // ...
        let old_state = include_str!("data/v2.0.0_state.json");
        let temp_dir = TempDir::new()?;
        let temp_path = temp_dir.path();

       // Open v0.12.2 file and write old state to it.
       let temp_state_path = temp_dir.path().join("state.json");
       let mut file = File::create(&temp_state_path)?;
       file.write_all(old_state.as_bytes())?;
        // ...
    }

Does it seem okay if this test can be updated so that the comparison against the old JSON state file check only for required keys being a match?

from pueue.

We can also make these optional columns that can be included once the query/filter syntax lands :)

Could be an interesting thing for some users.

from pueue.

ping @dmgolembiowski

Do you still plan to work on this :)? I intend to do a release in the upcoming weeks and this is something I would like to see in that release^^. In case you're no longer interested, I'll probably continue working on this :)

Whoops! Sorry I, I've been busy with other things and forgot about this.

from pueue.

Apologies if we're briefly losing commits due to an error I recently encountered:

╭─david at david-laptop in /home/david
╰─λ git clone https://github.com/dmgolembiowski/pueue/
Cloning into 'pueue'...
remote: fatal: bad tree object e0f56fb8b0421f874b4edd5c227912d219b669d9
remote: aborting due to possible repository corruption on the remote side.
fatal: early EOF
fatal: fetch-pack: invalid index-pack output

from pueue.

Pardon, @Nukesor , could you peek at github.com/dmgolembiowski/pueue/ and let me know what kinds of tests you'd like to integrate. As it's currently implemented, all tests are passing on nightly. (woo)

from pueue.

One more thing we should probably test:

Restarting tests without the --in-place option will also create a new (cloned) task.

One other thing I'm not sure about:
Should editing a task's command/path update the created_at field?

@mjpieters I would like to get your opinion on this as well :).

from pueue.

Ping @dmgolembiowski

How is it going :)? In case you are no longer working on this, I'll continue to work on this :)
This should definitely be in the next release. I'm going on holiday next week and I'll try to polish everything for a 3.0 release during that time.

Awesome, sounds good to me.

from pueue.