[services] Dev mode - Set up Backup service
ClosedPublic
Actions

Authored by • karol on Apr 11 2022, 6:01 AM.

Details

Reviewers

tomek
• jim

Commits

rCOMM6773029cb66b: [services] Dev mode - Set up Backup service

Summary

Depends on D3694

Modify the code of the backup service so the dev mode that uses the local cloud can be properly launched

Test Plan

cd services
docker-compose down
yarn run-local-cloud
yarn test-backup-service-dev-mode

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

• karol created this revision.Apr 11 2022, 6:01 AM

Herald added subscribers: • abosh, • benschac, atul and 3 others. · View Herald TranscriptApr 11 2022, 6:01 AM

Harbormaster completed remote builds in B8059: Diff 11278.Apr 11 2022, 6:06 AM

• karol requested review of this revision.Apr 11 2022, 6:06 AM

• karol edited the summary of this revision. (Show Details)Apr 11 2022, 6:37 AM

• karol edited the test plan for this revision. (Show Details)

• karol added reviewers: tomek, • jim.

• karol added a parent revision: D3694: [services] Dev mode - Add scripts for running local cloud.

update

• karol added a child revision: D3691: [services] Dev mode - Set up Blob service.Apr 11 2022, 6:38 AM

• karol added inline comments.Apr 11 2022, 6:47 AM

services/backup/docker-server/contents/server/src/AwsTools.cpp
10 ↗	(On Diff #11290)	@jimpo suggested avoiding using a compile-time flag https://phabricator.ashoat.com/D3498#98870 Using such a flag also occurs in `Constants.h` for both blob/backup - `COMM_TEST_SERVICES`. So if we want to stop using such variables, we should do this separately for both variables I think. @jimpo as you started this discussion: what do you think about reading env vars? For example with `getenv`?

• karol mentioned this in D3691: [services] Dev mode - Set up Blob service.Apr 11 2022, 6:48 AM

Harbormaster completed remote builds in B8071: Diff 11290.Apr 11 2022, 6:49 AM

@jimpo is on a lot of diffs and doesn't always have time for all of them, so to make sure he sees this one I'm setting him as blocking

• jim added inline comments.Apr 18 2022, 11:53 PM

services/backup/docker-server/contents/server/src/AwsTools.cpp
10 ↗	(On Diff #11290)	Reading an environment variable at runtime is a fine way to do it.

As noted, COMM_SERVICES_DEV_MODE should not be a compile-time flag, please fix

This revision now requires changes to proceed.Apr 26 2022, 9:41 AM

In D3690#107078, @jimpo wrote:

As noted, COMM_SERVICES_DEV_MODE should not be a compile-time flag, please fix

I don't it is a good idea to change this here. As I mentioned:

Using such a flag also occurs in Constants.h for both blob/backup - COMM_TEST_SERVICES. So if we want to stop using such variables, we should do this separately for both variables I think.

This is a typical situation when we need to do a follow-up because the change you're requesting applies in multiple places, not only this one. If we change it here, the codebase will be inconsistent. And if we change the rest of the occurrences here as well, the diff will be invalid as it will consist of unrelated changes.

I put up a diff with this follow-up D3872.