Paths

Table of Contentst

[services] Tunnelbroker - Moving the config file to the `~/.config` directory and removing the sandbox config
ClosedPublic
Actions

Authored by • max on Dec 1 2022, 8:35 AM.

Details

Reviewers

• jon
varun
bartek
tomek
ashoat

Group Reviewers

Restricted Owners Package

(Owns No Changed Paths)

Commits

rCOMM824a62323ca2: [services] Tunnelbroker - Moving the config file to the `~/.config` directory…

Summary

Moving the config file for Tunnelbroker to the ~/.config directory to unify services configuration. Removing using the "sandbox" config file. Updating services documentation about the new path and removing the sandbox config.

Linear task and full context: ENG-1680

Test Plan

Tunnelbroker service is successfully built.
Config file is successfully read from the new path.

Diff Detail

Repository

rCOMM Comm

Branch

move-config-file

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

• max created this revision.Dec 1 2022, 8:35 AM

• max held this revision as a draft.

Herald added a reviewer: • jon. · View Herald TranscriptDec 1 2022, 8:35 AM

Herald added subscribers: atul, tomek, ashoat. · View Herald Transcript

Harbormaster completed remote builds in B13918: Diff 19073.Dec 1 2022, 8:51 AM

• max published this revision for review.Dec 1 2022, 11:48 AM

• max edited the summary of this revision. (Show Details)

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

• max added reviewers: varun, bartek. • max added 1 blocking reviewer(s): • jon.

Should also remove the corresponding comm::network::tools::isSandbox() method if it's going to exist within the config file.

This revision is now accepted and ready to land.Dec 2 2022, 9:15 AM

AFAIK the comm::network::tools::isSandbox() is still used in a few other places

In D5790#173107, @jon wrote:

Should also remove the corresponding comm::network::tools::isSandbox() method if it's going to exist within the config file.

It's more complex and used in other C++ services. The task of removing isSandbox is tracked by the ENG-2296.

• max added a reviewer: tomek.Dec 2 2022, 10:11 AM

This revision now requires review to proceed.Dec 2 2022, 10:11 AM

Adding @ashoat because the documentation is changed.

docs/dev_services.md
42–43 ↗	(On Diff #19073)	Is it a good practice to use this directory? Is it possible that we will have some conflicts with other tools which might decide to use the same directory name?
59 ↗	(On Diff #19073)	This doesn't read well. Is this sentence inverted in some way?
81 ↗	(On Diff #19073)	Is it a good idea to just delete this part? Is it no longer relevant, or should be just updated?

This revision now requires changes to proceed.Dec 5 2022, 4:42 AM

ashoat added a reviewer: Restricted Owners Package.Dec 5 2022, 6:23 AM

ashoat added inline comments.

docs/dev_services.md
59 ↗	(On Diff #19073)	Yeah this needs to be fixed up

ashoat requested changes to this revision.Dec 5 2022, 6:23 AM

• max planned changes to this revision.Dec 5 2022, 8:56 AM

• max marked an inline comment as done.

Fix configuration file template sentence in a doc.

docs/dev_services.md
42–43 ↗	(On Diff #19073)	Is it a good practice to use this directory? Is it possible that we will have some conflicts with other tools which might decide to use the same directory name? We have a discussion with @jon about the flexibility of services configuration and `~/.config/$service.ini` seems a good way to go on. Not seeing any conflicts here.
59 ↗	(On Diff #19073)	This doesn't read well. Is this sentence inverted in some way? Hope it reads better now.
81 ↗	(On Diff #19073)	Is it a good idea to just delete this part? Is it no longer relevant, or should be just updated? We didn't remove the sandbox yet. We've removed usage of the `tunnelbroker-dev` config file and use a single config file. That's why I've removed only the part.

Harbormaster completed remote builds in B13998: Diff 19171.Dec 6 2022, 5:31 AM

Is it possible for anybody else to rewrite @max's English before I take a pass? Please re-add me once it's ready to go

In D5790#173953, @ashoat wrote:

Is it possible for anybody else to rewrite @max's English before I take a pass? Please re-add me once it's ready to go

I think @varun can take a quick look.

tomek accepted this revision.Dec 6 2022, 9:08 AM

tomek added 1 blocking reviewer(s): varun.

varun requested changes to this revision.Dec 6 2022, 7:27 PM

varun added inline comments.

docs/dev_services.md
59 ↗	(On Diff #19171)	Did you mean `services/tunnelbroker/tunnelbroker-sandbox.ini`? Also I don't think we need to mention this at all since the user is probably just going to copy-paste the above config, right?

This revision now requires changes to proceed.Dec 6 2022, 7:27 PM

Removing a sentence with the config file.

• max marked an inline comment as done.Dec 7 2022, 3:14 AM

• max added inline comments.

docs/dev_services.md
59 ↗	(On Diff #19171)	Did you mean `services/tunnelbroker/tunnelbroker-sandbox.ini`? Yes. `tunnelbroker-sandbox.ini` moved to `tunnelbroker.ini`. Also I don't think we need to mention this at all since the user is probably just going to copy-paste the above config, right? I think you're right. We can just remove it as the config will be copy pasted from the doc or created from the scratch for production. I've removed it.