Page MenuHomePhabricator

[services] Backup - Implement generating holder
ClosedPublic

Authored by karol on May 13 2022, 2:27 AM.
Tags
None
Referenced Files
F3252901: D4030.id12940.diff
Fri, Nov 15, 6:38 PM
Unknown Object (File)
Tue, Nov 5, 3:32 PM
Unknown Object (File)
Wed, Oct 23, 9:21 PM
Unknown Object (File)
Wed, Oct 23, 8:23 PM
Unknown Object (File)
Wed, Oct 23, 8:12 PM
Unknown Object (File)
Sun, Oct 20, 6:40 PM
Unknown Object (File)
Oct 16 2024, 8:43 AM
Unknown Object (File)
Oct 15 2024, 12:56 PM

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.May 13 2022, 2:27 AM
Harbormaster failed remote builds in B9081: Diff 12627!

services build passed

tomek requested changes to this revision.May 17 2022, 3:35 AM
tomek added inline comments.
services/backup/src/Tools.cpp
24–34 ↗(On Diff #12627)

If we modify the order of arguments we can have only one function with a default value.

Additionally, we can have the same schema for both cases, so that it's easier to parse it in the future (if we ever need to).

This revision now requires changes to proceed.May 17 2022, 3:35 AM
services/backup/src/Tools.cpp
24–34 ↗(On Diff #12627)

Good point, I missed that. But I think it will be more readable to have resourceID next to backupID, so we could do:

std::string generateHolder(
    const std::string &blobHash,
    const std::string &backupID,
    const std::string &resourceID = ""
    ) {
tomek requested changes to this revision.May 18 2022, 2:44 AM
tomek added inline comments.
services/backup/src/Tools.cpp
28–33 ↗(On Diff #12837)

Additionally, we can have the same schema for both cases, so that it's easier to parse it in the future (if we ever need to).

What do you think about the second part of my comment where I suggested that we should have the same number of sections in id, even when resourceID is empty?

This revision now requires changes to proceed.May 18 2022, 2:44 AM
services/backup/src/Tools.cpp
28–33 ↗(On Diff #12837)

At first I thought that aaa::bbb would look weird with :: but the argument about the parsing makes sense. Sorry I didn't implement this right away. I'm going to change this. Thanks.

remove ID_SEPARATOR from here

fix invalid update, generate the same holder no matter if the resourceID is empty or not

This revision is now accepted and ready to land.May 20 2022, 4:24 AM