| *** mhen_ is now known as mhen | 02:30 | |
| opendevreview | Dmitriy Rabotyagov proposed openstack/freezer master: Use snapshot timestamp for storage package naming https://review.opendev.org/c/openstack/freezer/+/975380 | 14:23 |
|---|---|---|
| opendevreview | Dmitriy Rabotyagov proposed openstack/freezer master: Return restore request if no backups for volume https://review.opendev.org/c/openstack/freezer/+/975652 | 16:04 |
| noonedeadpunk | mhen: I added handling of the max() issue in here ^ | 16:04 |
| mhen | noonedeadpunk: if freezer-scheduler invokes freezer-agent here and reaches this case, wouldn't the agent return with a zero exit code and freezer-scheduler misinterpret this job as succeeded if it simply returns at that point? | 16:06 |
| noonedeadpunk | do you think we should raise there? | 16:07 |
| noonedeadpunk | I don't actually see that anybody is checking for the even there | 16:07 |
| noonedeadpunk | but yeah, that is a good point | 16:07 |
| mhen | definitely; even if using freezer-agent directly on the CLI as a user I'd expect an error message being returned that no applicable backup could be found | 16:07 |
| mhen | this way it would silently fail in either case | 16:08 |
| mhen | and I think this is bad UX | 16:08 |
| mhen | from a user's POV, I want it to fail hard and loud if it was unable to do the thing it said it would do (in this case: restore a backup) | 16:09 |
| noonedeadpunk | I kinda wonder how we allow `volume_id` to be None | 16:10 |
| mhen | where? | 16:12 |
| noonedeadpunk | I think another raise should be there if both volume_id and backup_id are None | 16:12 |
| noonedeadpunk | https://github.com/openstack/freezer/blob/a3210ca85aa698e0af957dd85b41609c6522261e/freezer/openstack/restore.py#L220 | 16:13 |
| mhen | https://github.com/openstack/freezer/blob/master/freezer/common/config.py#L790-L791 | 16:15 |
| mhen | it's because you must have either to even trigger that code path | 16:16 |
| mhen | and "--mode cindernative" doesn't actually do anything | 16:17 |
| mhen | I have used "--mode cinder" with "--cindernative-vol-id" and triggered the cindernative mode | 16:17 |
| mhen | even though "--mode cindernative" is a documented choice | 16:17 |
| mhen | it is irrelevant | 16:17 |
| mhen | because of the "backup_media" stuff linked above | 16:18 |
| mhen | it is a mess at times to be honest | 16:18 |
| noonedeadpunk | yeah, it looks like it... | 16:18 |
| opendevreview | Merged openstack/freezer master: Use snapshot timestamp for storage package naming https://review.opendev.org/c/openstack/freezer/+/975380 | 16:27 |
| opendevreview | Merged openstack/freezer master: Fix restore-from-date behavior https://review.opendev.org/c/openstack/freezer/+/975437 | 16:48 |
| opendevreview | Dmitriy Rabotyagov proposed openstack/freezer master: Return restore request if no backups for volume https://review.opendev.org/c/openstack/freezer/+/975652 | 16:49 |
| opendevreview | Dmitriy Rabotyagov proposed openstack/freezer master: Return restore request if no backups for volume https://review.opendev.org/c/openstack/freezer/+/975652 | 18:05 |
Generated by irclog2html.py 4.0.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!