api: Add snap info to each task #13902
Conversation
github-actions
bot
added
the
Needs Documentation -auto-
In order to allow Refresh Awareness to be able to link a task to the corresponding snap being refreshed when a single notice contains several snaps, it is a must to include that info into each task. This patch adds the snap name, snap instance and snap revision (when available) into each task.
sergio-costas
force-pushed
the
UDENG-2282-add-snap-info-in-each-task
branch
from
78f76ac
to
0befff6
Compare
sergio-costas
changed the title
pedronis
added
the
Needs Samuele review
|
This is only required if a change sent through the notices interface to inform about a snap being auto-refreshed after being closed by the user, can hold tasks for more than one snap. If it is guaranteed that only one snap is assigned to a change, then this isn't needed. |
| SnapName string `json:"snap-name,omitempty"` | ||
| InstanceName string `json:"instance-name,omitempty"` | ||
| Revision string `json:"revision,omitempty"` |
There was a problem hiding this comment.
we don't want to add this kind of data this way. We probably want to add support instead for something like api-data that we have for Changes. Also (even) if we do that the closest thing we can do that doesn't feel too forced would be to expose a list of instance names "affected-snaps" that matches the same concept in snapstate conflict logic, we would have to expose some of that to daemon
There was a problem hiding this comment.
Separately, how are the consumers expected to use this information? Say if a change touches more than one, would they list all the tasks, and then continue observing which ones for which snap are done?
There was a problem hiding this comment.
You can see a video with what I have right now: https://warthogs.atlassian.net/browse/UDENG-2282 , in the most recent comment. As you can see, the idea is to have a progress bar for each snap being updated, and that requires to know how many tasks are assigned to that snap refresh, and how many are already done. Currently I'm presuming that there is one change per snap, so each change has only one snap in its snap_names property, and thus all the tasks belong to that snap. But if there are several snaps in the same change, then each progress bar will show the same progress, unless this patch (or something similar) is implemented, that allows me to separate the tasks.
There was a problem hiding this comment.
Also, here is an screen capture
There was a problem hiding this comment.
unclear that "snap-tasks" is a good name for it, so we don't have a good name, also it's not an obvious approach except is compact
There was a problem hiding this comment.
also the obvious way to attach the information in snap-tasks form would be when we set snap-names, but actually a Change can grow more tasks after that point
There was a problem hiding this comment.
Good point... if the task list can grow, then it makes sense to include this info in the task.
There was a problem hiding this comment.
but actually a Change can grow more tasks after that point
to reiterate on this, snap can pull prerequisites (like a new base) or a re-refresh may occur, at which point the task list will grow again, so any progress bar the user sees will need to be adjusted such that it doesn't jump backwards
There was a problem hiding this comment.
Anyway, as I said in the top, if it is guaranteed that, when doing a refresh after the user closes the application, only one snap is associated with each change, this won't be needed. I tried killing two applications "at the same time" using kill XXXXX && kill YYYYY, and still receive two changes, one per snap...
There is still another scenario where snap run of a certain snap is inhibited from running due to a normal refresh/auto-refresh that includes that snap and the progress UI should only be relevant to the snap that is being started and not the other snaps in the auto-refresh. It not just when running the snap after closing.
|
we agreed on a different approach, matching more how this works for Changes, as discussed in the comments here. @ZeyadYasser will work on that. Closing this one |
In order to allow Refresh Awareness to be able to link a task to the corresponding snap being refreshed when a single notice contains several snaps, it is a must to include that info into each task.
This patch adds the snap name, snap instance and snap revision (when available) into each task.
This is only required if a change sent through the notices interface to inform about a snap being auto-refreshed after being closed by the user, can hold tasks for more than one snap. If it is guaranteed that only one snap is assigned to a change, then this isn't needed.
Thanks for helping us make a better snapd!
Have you signed the license agreement and read the contribution guide?