Details
- Reviewers
tomek ashoat - Commits
- rCOMMb5fa77b9658a: [services] Backup - generate backup id
Run the blob service
Run the backup service
Use grpc-playground to make a call creating a new backup
Either log out the generated backup id or check in the database
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| services/backup/src/Reactors/server/CreateNewBackupReactor.cpp | ||
|---|---|---|
| 12–15 ↗ | (On Diff #12625) | We're not using userID so why do we have it as a parameter? What's the point of having both UUID and timestamp when timestamp is a part of UUID? |
| services/backup/src/Reactors/server/CreateNewBackupReactor.cpp | ||
|---|---|---|
| 12–15 ↗ | (On Diff #12625) | Good point, I'll remove it. Ok, if so, we can have backupID == generateUUID |
Adding @ashoat because choosing id schema is an important decision
| services/backup/src/Reactors/server/CreateNewBackupReactor.cpp | ||
|---|---|---|
| 12–15 ↗ | (On Diff #12625) | It is described in a couple of places, e.g. https://en.wikipedia.org/wiki/Universally_unique_identifier#Format But in the future, please try to research it yourself and verify if the proposition is valid. |
| services/lib/src/GlobalTools.h | ||
| 9 ↗ | (On Diff #12836) | After this change we no longer need this in this diff. It is needed though later in the stack. |
| services/backup/src/Reactors/server/CreateNewBackupReactor.cpp | ||
|---|---|---|
| 12–15 ↗ | (On Diff #12625) | I was looking in the boost docs as we use boost and couldn't find it. Essentially, we use boost's code for this so maybe a more solid source would be to provide something from the docs/code? Nevertheless, I suppose we can assume that the term UUID means the same for Wikipedia's article author and boost authors.
Why? I implemented some code, and you proposed this change. So effectively, you proposed some code here. Shouldn't that be you in this case who backs up their idea? |
| services/lib/src/GlobalTools.h | ||
| 9 ↗ | (On Diff #12836) | Sure, I can move this. |
| services/backup/src/Reactors/server/CreateNewBackupReactor.cpp | ||
|---|---|---|
| 12–15 ↗ | (On Diff #12625) |
Think about this in this way: your diff is a proposition of solution. I have some doubts about it. So you should prove that my doubts are invalid. So my idea was the result of doubts I have about your idea. |
I'm open to this idea, but I think there's been a slight misinterpretation of my Linear comment here. I was suggesting that we use a UUID for the device ID, but it would be the same UUID for the device (persisted in Redux, for instance). And then the backup would be <device_id>:<timestamp>, where the device ID is a UUID, and the timestamp is the current timestamp.
I think Karol interpreted my comment as suggesting that we use a UUID for the whole backup in place of the device ID. So it's basically <backup_uuid>:<timestamp>.
I think @palys-swm's suggestion to remove the timestamp makes sense for Karol's interpretation, but not for mine.
One potential advantage of mine might be that it might be easier to group backups by device ID (in a batch job, for instance), and another is that it may be possible to force an efficient index that shows you all of the backups for a given device (not sure if DynamoDB supports this).
What do you all think?
I don't mind and I can implement any solution, really. So in the case you're describing, the device_id is just something we're going to get from the client, right?
I added a diff before this one that reads the device id from the client, so we can use it here D4099