Move CorrelationID up a level out of Target #108
Conversation
cajun-rat
force-pushed
the
correlation-id
branch
from
b640430 to
4f70cef
Compare
| @@ -1119,7 +1120,7 @@ void SQLStorage::saveInstalledVersion(const std::string& ecu_serial, const Uptan | |||
| auto statement = db.prepareStatement<std::string, int, int, int64_t>( | |||
| "UPDATE installed_versions SET correlation_id = ?, is_current = ?, is_pending = ?, was_installed = ? WHERE id " | |||
| "= ?;", | |||
| target.correlation_id(), static_cast<int>(update_mode == InstalledVersionUpdateMode::kCurrent), | |||
| correlation_id, static_cast<int>(update_mode == InstalledVersionUpdateMode::kCurrent), | |||
There was a problem hiding this comment.
@detsch I think this change will break us, so we will need to adjust the fio's client (aktualizr-lite) to this change.
There was a problem hiding this comment.
Also, it may lead to different correlation ID usage for the same target. For example, a client specifies corID-1 when storing a target as pending for the first time, and then the client, by mistake, may set corID-2 when updating the target status/mode to kCurrent.
There was a problem hiding this comment.
@cajun-rat Do we really need to set the correlation_id field if it's just an update of an existing record? I mean, do we need to update/change the correlation_id in this case at all, considering that only the is_* and was_* fields need to be updated?
The Correlation ID logically belongs to a particular update that is assigned by the server, and not to a 'target', which is one of the things to be installed. Move the information about the current correlation id into the director, and explictly persist it in InvStorage. Eventually I think we need to move torward a model where we have an explicit representation of an 'installation job'--including a correlation id--which persists over reboots. At the moment we infer this from the 'pending' flag for individual ECU installations.
This avoids a ", nullptr)" everywhere. See discussion in uptane/aktualizr#108 This is implemented as a non-virtual member function on the base class, because mixing default parameters and virtual calls is dangerous.
|
Thank you @mike-sul ! |
The Correlation ID logically belongs to a particular update that is assigned by the server, and not to a 'target', which is one of the things to be installed. Move the information about the current correlation id into the director, and explictly persist it in InvStorage.
Eventually I think we need to move torward a model where we have an explicit representation of an 'installation job'--including a correlation id--which persists over reboots. At the moment we infer this from the 'pending' flag for individual ECU installations.