Depends on D3466
Add implementation for the pull backup reactor
Details
Details
cd services yarn run-backup-service
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| services/backup/docker-server/contents/server/src/Reactors/server/PullBackupReactor.h | ||
|---|---|---|
| 25–31 ↗ | (On Diff #10644) | Is there a reason why this implementation is in the header file? |
| services/backup/docker-server/contents/server/src/Reactors/server/PullBackupReactor.h | ||
|---|---|---|
| 25–31 ↗ | (On Diff #10644) | Agree that we should avoid implementations in .h files when possible. If it's addressed in a later diff, would be good to reference that here (or just keep move this stub to a .cpp file to avoid any confusion) |
Comment Actions
It is accepted as a draft so I removed the draft label and I'm requesting changes once again to keep the appropriate order of events.
| services/backup/docker-server/contents/server/src/Reactors/server/PullBackupReactor.h | ||
|---|---|---|
| 25–31 ↗ | (On Diff #10644) | Yes, I was getting weird and frustrating compile errors with templates in the background. Maybe this code can be moved to a CPP file. I don't mind as long as it works. Can we do this separately? https://linear.app/comm/issue/ENG-917/try-to-move-reactors-implementations-to-cpp-files |
| services/backup/docker-server/contents/server/src/Reactors/server/PullBackupReactor.h | ||
|---|---|---|
| 16 | May be an oversight. Even with auth, we needed a simple auth in this case. I think we can change this into a write reactor but maybe in a separate diff since it requires a proto file change. | |
| services/backup/docker-server/contents/server/src/Reactors/server/PullBackupReactor.h | ||
|---|---|---|
| 16 | ||