Depends on D4033
In order to Be able to send attachments for certain logs, we should first get new logs ids from the backup service.
Details
Details
- terminal 1
cd services yarn run-blob-service-dev-mode
- terminal 2
cd services yarn run-backup-service-dev-mode
- terminal 3
git clone https://github.com/karol-bisztyga/grpc-playground.git cd grpc-playground git checkout backup-async ./build.sh ./cmake/build/bin/client
Then in terminal 3 create a new backup
n
Then in terminal 3 create a new log
l
And see that you obtained a new log id.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| native/cpp/CommonCpp/grpc/protos/backup.proto | ||
|---|---|---|
| 55 ↗ | (On Diff #12918) | Is the logID a timestamp? I wonder if it's better to be more specific here and call this logCheckpoint or logTimestamp or something |
| services/backup/src/Reactors/server/CreateNewBackupReactor.cpp | ||
| 94–95 ↗ | (On Diff #12918) | Agree, looks like something may have gone wrong with Git here? |
| native/cpp/CommonCpp/grpc/protos/backup.proto | ||
|---|---|---|
| 55 ↗ | (On Diff #12918) | Log id != log timestamp. std::string SendLogReactor::generateLogID(const std::string &backupID) {
return backupID + tools::ID_SEPARATOR +
std::to_string(tools::getCurrentTimestamp());
}I'm not going to do any updates here as you were wrong with your assumption. If you still think the naming can be better here, let me know. Nevertheless, I think the naming is at its best here. We have a function generateLogID and then we want to return it to the client. It is stored in the DB as the logID. Why adding such a confusion here? |
| native/cpp/CommonCpp/grpc/protos/backup.proto | ||
|---|---|---|
| 56 ↗ | (On Diff #13045) | Thanks for clarifying! I can see it's not a timestamp, but it is monotonically increasing, so it might count as a "checkpoint". I guess in this case we can treat the SendLog as an independent operation, but if/when we do have multiple logs queued up, it might make sense to checkpoint their upload, just like we checkpoint their download. |
| native/cpp/CommonCpp/grpc/protos/backup.proto | ||
|---|---|---|
| 56 ↗ | (On Diff #13045) | Ok, so from what I understand, despite the inconsistency in naming, you still think we should have logCheckpoint here. |