[services] Backup - Remove backup base Docker image
ClosedPublic
Actions

Authored by • karol on Mar 21 2022, 1:35 PM.

Details

Reviewers

ashoat
• jim
tomek

Commits

rCOMMa5d1993c343c: [services] Backup - Remove backup base Docker image

Summary

Remove base Docker image for backup service and simplify directory structure.

Analogous changes to D3481 and D3234.

Depends on D3778

https://linear.app/comm/issue/ENG-771/service-base-image-cache-inefficiency

Test Plan

cd services
yarn run-backup-server

Diff Detail

Repository

rCOMM Comm

Branch

backup-blob

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

• jim created this revision.Mar 21 2022, 1:35 PM

Herald added subscribers: • benschac, atul, • adrian, tomek. · View Herald TranscriptMar 21 2022, 1:35 PM

Harbormaster returned this revision to the author for changes because remote builds failed.Mar 21 2022, 1:36 PM

Harbormaster failed remote builds in B7518: Diff 10583!

Harbormaster completed remote builds in B7518: Diff 10583.Mar 22 2022, 8:21 AM

• jim requested review of this revision.Mar 22 2022, 8:21 AM

Accepting, but I want to make sure @karol-bisztyga takes a look too since he has the most context

Acceptable, but please rebase to the current master and make sure that the backup service builds as there are conflicts. Passing back to you.

This revision now requires changes to proceed.Mar 28 2022, 4:45 AM

Rebased

Harbormaster completed remote builds in B7850: Diff 11023.Apr 4 2022, 2:26 PM

@karol-bisztyga can you review this please? It's been a couple days, and this is the kind of diff where you have to painfully rebase whenever there is a delay...

Can we wait until the stack @ D3488 is landed? We waited a week for that rebase and during this week I made a lot of progress with that stack. Now, this diff is all about refactoring so it doesn't take much to rename a couple of directories, but if I wanted to rebase my stack to this diff, I'd probably need to resolve a lot of conflicts, etc.

• karol requested changes to this revision.Apr 6 2022, 11:57 PM

This revision now requires changes to proceed.Apr 6 2022, 11:57 PM

Commandeering as discussed with @jimpo

This revision is now accepted and ready to land.Apr 19 2022, 12:56 AM

rebase to my stack

rebase

This revision is now accepted and ready to land.Apr 19 2022, 3:19 AM

• karol edited reviewers, added: tomek; removed: • max. • karol added 1 blocking reviewer(s): • jim.Apr 19 2022, 3:20 AM

This revision now requires review to proceed.Apr 19 2022, 3:20 AM

• karol edited the summary of this revision. (Show Details)Apr 19 2022, 3:21 AM

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

• karol added a parent revision: D3778: [services] Backup - Improve code style in send log reactor.

• karol retitled this revision from [services] Remove backup base Docker image to [services] Backup - Remove backup base Docker image.

Harbormaster completed remote builds in B8302: Diff 11598.Apr 19 2022, 3:26 AM

• karol added a child revision: D3779: [services] Backup - Fix: add missing pragma once to headers.Apr 19 2022, 5:12 AM

The sooner we can get this landed, the better!

• karol edited the summary of this revision. (Show Details)Apr 19 2022, 11:55 PM

• karol mentioned this in D3777: [services] Backup - Improve code style in create new backup reactor.Apr 20 2022, 5:22 AM

I don't have too much context, but it looks ok.

The test plan should include something about formatting by clang-format. We had a lot of issues with something not being formatted properly after being moved, so checking if the code is properly formatted should be a part of the test plan.

• karol planned changes to this revision.Apr 22 2022, 3:42 AM

update

@palys-swm clang's fixed in D3802

Harbormaster completed remote builds in B8444: Diff 11783.Apr 22 2022, 3:57 AM

Hey @jimpo is there any ETA for a review here? It's blocking.

Thanks!

• jim accepted this revision.Apr 26 2022, 9:40 AM

This revision is now accepted and ready to land.Apr 26 2022, 9:40 AM

Closed by commit rCOMMa5d1993c343c: [services] Backup - Remove backup base Docker image (authored by • karol). · Explain WhyApr 27 2022, 2:37 AM

This revision was automatically updated to reflect the committed changes.

• karol added a commit: rCOMMa5d1993c343c: [services] Backup - Remove backup base Docker image.

• karol mentioned this in D3695: [services] Dev mode - Fix Backup service.Apr 27 2022, 3:14 AM