Page MenuHomePhabricator
Differential D3788

[services] Blob - Pull more mutual code from base reactors - Add base reactor
ClosedPublic
Actions

Authored by • karol on Apr 20 2022, 4:01 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Mar 9, 8:45 PM
Unknown Object (File)
Sun, Mar 9, 8:18 AM
Unknown Object (File)
Sun, Mar 9, 8:18 AM
Unknown Object (File)
Sun, Mar 9, 8:18 AM
Unknown Object (File)
Sun, Mar 9, 8:18 AM
Unknown Object (File)
Sun, Mar 9, 8:18 AM
Unknown Object (File)
Sun, Mar 9, 8:13 AM
Unknown Object (File)
Thu, Mar 6, 7:04 PM
View All Files
Subscribers
• abosh
• adrian
ashoat
atul
• benschac
tomek

Details

Reviewers
tomek
ashoat
Commits
rCOMM875fbc6e1c9d: [services] Blob - Pull more mutual code from base reactors - Add base reactor
Summary

Depends on D3787

https://linear.app/comm/issue/ENG-916/consider-pulling-more-mutual-code-from-the-async-reactors

Adding a master base reactor for all reactors to the blob service

This is the same code as in D3785

Test Plan
cd services
yarn run-blob-service

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

• karol created this revision.Apr 20 2022, 4:01 AM
Herald added subscribers: • abosh, • benschac, atul and 3 others. · View Herald TranscriptApr 20 2022, 4:01 AM
• karol edited the summary of this revision. (Show Details)Apr 20 2022, 4:04 AM
• karol edited the test plan for this revision. (Show Details)
• karol added a reviewer: tomek.
• karol added a parent revision: D3787: [services] Backup - Pull more mutual code from base reactors - Apply in server reactors.
Harbormaster completed remote builds in B8348: Diff 11658.Apr 20 2022, 4:05 AM
• karol requested review of this revision.Apr 20 2022, 4:05 AM
• karol added a child revision: D3789: [services] Blob - Pull more mutual code from base reactors - Apply in server reactors.Apr 20 2022, 4:21 AM
• karol mentioned this in D3719: [services] Backup - client reactors - distinguish terminated from done.
ashoat added a comment.Apr 20 2022, 10:56 AM

This is just a copy of D3785... wondering, can we share the BaseReactor definition across services, or is there some reason that it has to be different?

• karol added a comment.Apr 22 2022, 5:11 AM

This is a part of https://linear.app/comm/issue/ENG-324/avoid-repetitions-in-services-code

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

Sorry, added a note to the description that it is a copy

• karol planned changes to this revision.Apr 22 2022, 5:12 AM
• karol updated this revision to Diff 11789.Apr 22 2022, 5:13 AM

rebase

Harbormaster completed remote builds in B8450: Diff 11789.Apr 22 2022, 5:17 AM
ashoat requested changes to this revision.Apr 22 2022, 11:18 AM

See my question above: can we share the BaseReactor definition across services, or is there some reason that it has to be different? (Will request changes this time to make sure it's noticed)

This revision now requires changes to proceed.Apr 22 2022, 11:18 AM
• karol updated this revision to Diff 12041.Apr 28 2022, 7:04 AM

new version of stack

Harbormaster completed remote builds in B8650: Diff 12041.Apr 28 2022, 7:17 AM
ashoat requested changes to this revision.Apr 28 2022, 11:45 AM

can we share the BaseReactor definition across services, or is there some reason that it has to be different?

In D3788#105937, @ashoat wrote:

See my question above: can we share the BaseReactor definition across services, or is there some reason that it has to be different? (Will request changes this time to make sure it's noticed)

Not sure if this is addressed on another diff, but I don't see an answer here...

This revision now requires changes to proceed.Apr 28 2022, 11:45 AM
• karol requested review of this revision.Apr 29 2022, 4:39 AM

There is a task for this https://linear.app/comm/issue/ENG-1035/make-sure-the-base-reactor-is-not-copy-pasted-across-multiple-services

tomek accepted this revision.Apr 29 2022, 5:40 AM

I have the same questions as in D3785.

Also, I think that prioritizing code sharing is a really good idea.

ashoat accepted this revision.Apr 30 2022, 4:10 PM

Cool thanks, let's follow-up on the task

This revision is now accepted and ready to land.Apr 30 2022, 4:10 PM
Closed by commit rCOMM875fbc6e1c9d: [services] Blob - Pull more mutual code from base reactors - Add base reactor (authored by • karol). · Explain WhyMay 5 2022, 1:47 AM
This revision was automatically updated to reflect the committed changes.
• karol added a commit: rCOMM875fbc6e1c9d: [services] Blob - Pull more mutual code from base reactors - Add base reactor.

Revision Contents
Changeset List

PathSize
services/
blob/
1 line
src/
Reactors/
31 lines

Diff 11789

View Options

services/blob/CMakeLists.txt

  • This file was added.
PropertyOld ValueNew Value
File Modenull100755
#!/usr/bin/env bash
# This file exists to make a smaller docker context, so that building it is
# significantly faster and requires less system resources
SCRIPT_DIR="$(cd "$(dirname "$0")" || exit 1; pwd -P)"
BUILD_DIR="${SCRIPT_DIR}/target/oci_image"
rm -rf "$BUILD_DIR"
mkdir -p "$BUILD_DIR"/{scripts,shared,services/tunnelbroker}
cp "$SCRIPT_DIR/../../scripts/install_protobuf.sh" "$BUILD_DIR"/scripts
cp -r "${SCRIPT_DIR}/../../shared/protos" "$BUILD_DIR"/shared/protos
cp -r "${SCRIPT_DIR}/../../shared/tunnelbroker_messages" \
"$BUILD_DIR"/shared/tunnelbroker_messages
cp -r "${SCRIPT_DIR}"/{Cargo.toml,Cargo.lock,build.rs,src} \
"$BUILD_DIR"/services/tunnelbroker/
docker build -f "${SCRIPT_DIR}/Dockerfile" "$BUILD_DIR"
View Options

services/blob/src/Reactors/BaseReactor.h

  • This file was added.
PropertyOld ValueNew Value
File Modenull100755
#!/usr/bin/env bash
# This file exists to make a smaller docker context, so that building it is
# significantly faster and requires less system resources
SCRIPT_DIR="$(cd "$(dirname "$0")" || exit 1; pwd -P)"
BUILD_DIR="${SCRIPT_DIR}/target/oci_image"
rm -rf "$BUILD_DIR"
mkdir -p "$BUILD_DIR"/{scripts,shared,services/tunnelbroker}
cp "$SCRIPT_DIR/../../scripts/install_protobuf.sh" "$BUILD_DIR"/scripts
cp -r "${SCRIPT_DIR}/../../shared/protos" "$BUILD_DIR"/shared/protos
cp -r "${SCRIPT_DIR}/../../shared/tunnelbroker_messages" \
"$BUILD_DIR"/shared/tunnelbroker_messages
cp -r "${SCRIPT_DIR}"/{Cargo.toml,Cargo.lock,build.rs,src} \
"$BUILD_DIR"/services/tunnelbroker/
docker build -f "${SCRIPT_DIR}/Dockerfile" "$BUILD_DIR"