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

Hooks run on task import #3314

Closed
tyjak opened this issue Mar 30, 2024 · 4 comments
Closed

Hooks run on task import #3314

tyjak opened this issue Mar 30, 2024 · 4 comments

Comments

@tyjak
Copy link

tyjak commented Mar 30, 2024

  • What command(s) did you run?

task import all-tasks.json

  • What did you expect to happen?

Get all my tasks unchanged prior version 3.0.0

  • What actually happened?

I've got 1637 tasks to import

task import stop running and did not import all tasks : task count return 183
rerun task import import more tasks in skip the already imported ones and seems to import more, but I've done that multiple times.

skip 7cf582d1-8797-4b0c-91f2-f5025596b9cb script serveur pour transformer une liste de fichier en lien rss 
 skip c2ba656b-1927-4a45-9fbb-122e04947938 Add a feature to make password menu generate a pass if it's not existing
 skip a9c1c9dd-215c-442e-9d14-d26c909a9922 script serveur pour transformer une liste de fichier en lien rss
 skip 51dd0ca7-e60d-48e6-949b-079916b455dc Improve i3menu openvpn -> be sure the connection is initialized, put in orange when the connection is started but the sequence not yet fully initialized
 add  1c44ce70-e3b5-4a09-9fa8-a70879fca4be i3 : dernier écran -> aller à l'écran précédant
 add  2cb2f1a7-cfa1-4baf-9928-0077c62c3860 Utiliser MPRIS pour le controle audio
 add  0e3f658f-39d9-476f-8ebb-e596772d7c7b clipmenud crash on startup no way to start it after X11 see notes
 add  9de15063-2699-462d-b4f9-cab3f5aa8515 Docker service failed
 add  6fadf6eb-9ccf-4788-9380-1e148cd9384c Redshift service failed

During import process I've encountered this error :
jq: error (at <stdin>:1): null (null) cannot be tsv-formatted, only array

And finally I stopped the process when I realized that import was working with hooks that achieved to messed up everything.

It should be mentioned in the release doc upgrade that hooks are processed during the import process, and must be deactivated if not needed to re-modify your tasks again

Thank you for your fantastic work, I stick on task 2.6.2 for the moment, don't have too much time and enough confident to fix the problems I encountered.

@djmitche djmitche changed the title task import messed up when upgrade to 3.0 Hooks run on task import Mar 30, 2024
@djmitche djmitche moved this to 3.0-Fix in Taskwarrior Development Mar 30, 2024
@djmitche
Copy link
Collaborator

I didn't realize that was the case -- I know very little about hooks, actually. But, it does seem that hooks should not run on an import!

Since import is critical to upgrading to 3.x, I'll see if we can't fix this in the next release. I've added a note to the release and will also update the updating instructions.

@djmitche
Copy link
Collaborator

In GothenburgBitFactory/tw.org#789 I added rc.hooks=0 to the suggested command, so hopefully those reading the instructions from here on out won't be caught by this issue.

After looking at the code for hooks, I'm less confident that we should change this default right now, while fixing 3.0 things. Maybe people have workflows that expect hooks to run on import? @tbabej were you involved in the design of hooks? What do you think about disabling hooks for all task import commands?

@tbabej
Copy link
Member

tbabej commented Mar 31, 2024

I do remember the hooks discussions, currently the implementation does not differentiate between the command used that triggered the modification.

I don't think users rely on hooks running on import too much, although that interface is sometimes used to ingest JSON-formatted tasks from other sources / integrations. So it is not unfeasable someone uses it in that way.

That said I would be ok with disabling hooks by default and then observing if any users complain, or introducing an explicit setting hooks.run-on-import that would have to be set to true for hooks to run on import if desired.

@djmitche
Copy link
Collaborator

OK, thanks! Given that information, let's leave it as-is for now and just expect users to use rc.hooks=0 when importing on upgrade. That seems better than breaking more workflows at the moment!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

No branches or pull requests

3 participants