feat: Introduce top-level config package #389

orpheuslummis · 2022-05-03T17:00:58Z

Introducing a top-level config package in which a Config struct provides DefraDB's configuration, providing facilities such as a documented YAML file, ...

Some goals of this are:

top-level clear configuration
allow configuration via env. variables, CLI flags, and config file
extensible

I recommend starting a review with the config package documentation https://github.com/sourcenetwork/defradb/pull/389/files#diff-fe44f09c4d5977b5f5eaea29170b6a0748819c9d02271746a20d81a5f3efca17R11

Future extensions of this will be configuration for TLS, instrumentation, CORS, etc.

Feedback and discussion is welcome.

Note that the in-repo CLI documentation will be updated as part of the CLI overhaul PR that follows this one.

codecov · 2022-05-03T17:02:41Z

Codecov Report

Merging #389 (77de77c) into develop (3957381) will increase coverage by 0.28%.
The diff coverage is 68.10%.

@@             Coverage Diff             @@
##           develop     #389      +/-   ##
===========================================
+ Coverage    56.58%   56.87%   +0.28%     
===========================================
  Files          121      124       +3     
  Lines        14082    14351     +269     
===========================================
+ Hits          7969     8162     +193     
- Misses        5421     5475      +54     
- Partials       692      714      +22

Impacted Files	Coverage Δ
logging/config.go	`76.56% <ø> (ø)`
node/node.go	`64.46% <33.33%> (-1.67%)`	⬇️
node/config.go	`59.61% <40.90%> (ø)`
config/rootdir.go	`60.52% <60.52%> (ø)`
config/config.go	`72.64% <72.64%> (ø)`
config/configfile.go	`75.00% <75.00%> (ø)`

orpheuslummis · 2022-05-06T15:41:17Z

also closes #347

AndrewSisley

Was really nice to review - I really like the changes, I dont think I have any major comments (mostly ticket requests/questions/documentation). Approved assuming you respond to my comments before merge.

config/config.go

config/config_test.go

config/rootdir.go

node/config.go

cli/defradb/cmd/root.go

config/config.go

cli/defradb/cmd/init.go

node/config.go

cli/defradb/cmd/root.go

orpheuslummis · 2022-05-24T18:35:06Z

Closes #469

orpheuslummis · 2022-05-24T22:22:03Z

Closes #346

jsimnz

Sorry for the delay on this, some minor changes/suggestions

cli/defradb/cmd/init.go

shahzadlone

LGTM.

fredcarle

LGTM!

orpheuslummis · 2022-06-28T16:09:35Z

Sorry for having just one commit on this one, I didn't know any better

Main difference from previous iteration is the way CLI flags are handled solving a flag precedence issue

AndrewSisley

I skimmed through again, looks good with a couple of minor comments I would prefer sorted pre-merge. Thanks again for cleaning this up!

cli/serverdump.go

config/config.go

fredcarle

Approving provided you integrate the suggested change :)

node/config.go

orpheuslummis added the refactor This issue specific to or requires *notable* refactoring of existing codebases and components label May 3, 2022

orpheuslummis added this to the DefraDB v0.3 milestone May 3, 2022

orpheuslummis self-assigned this May 3, 2022

This comment was marked as off-topic.

Sign in to view

orpheuslummis changed the title ~~refactor: introduce top-level config package~~ refactor: Introduce top-level config package May 3, 2022

This comment was marked as off-topic.

Sign in to view

orpheuslummis mentioned this pull request May 6, 2022

feat: Configurable caller option of logger #416

Merged

orpheuslummis changed the title ~~refactor: Introduce top-level config package~~ feat: Introduce top-level config package May 6, 2022

orpheuslummis force-pushed the orpheus/refactor/config branch from 5fab950 to cd41cb9 Compare May 6, 2022 15:11

This comment was marked as off-topic.

Sign in to view

jsimnz added feature New feature or request and removed refactor This issue specific to or requires *notable* refactoring of existing codebases and components labels May 6, 2022

This comment was marked as off-topic.

Sign in to view

orpheuslummis added the action/no-benchmark Skips the action that runs the benchmark. label May 7, 2022

orpheuslummis force-pushed the orpheus/refactor/config branch from e976660 to ea50dc8 Compare May 10, 2022 15:35

orpheuslummis marked this pull request as ready for review May 10, 2022 15:36

orpheuslummis requested review from jsimnz, shahzadlone, AndrewSisley and fredcarle May 10, 2022 15:36

AndrewSisley approved these changes May 10, 2022

View reviewed changes

fredcarle reviewed May 11, 2022

View reviewed changes

cli/defradb/cmd/root.go Outdated Show resolved Hide resolved

shahzadlone reviewed May 11, 2022

View reviewed changes

config/config.go Outdated Show resolved Hide resolved

cli/defradb/cmd/init.go Outdated Show resolved Hide resolved

node/config.go Show resolved Hide resolved

fredcarle reviewed May 11, 2022

View reviewed changes

cli/defradb/cmd/root.go Outdated Show resolved Hide resolved

orpheuslummis force-pushed the orpheus/refactor/config branch from 6283b94 to 708bb3f Compare May 13, 2022 15:00

orpheuslummis mentioned this pull request May 24, 2022

Top-level config package #469

Closed

orpheuslummis linked an issue May 24, 2022 that may be closed by this pull request

Top-level config package #469

Closed

orpheuslummis linked an issue May 24, 2022 that may be closed by this pull request

Configuration overhaul #346

Closed

orpheuslummis mentioned this pull request May 25, 2022

feat: Add CORS capability to HTTP API #467

Merged

7 tasks

jsimnz requested changes Jun 3, 2022

View reviewed changes

cli/defradb/cmd/init.go Outdated Show resolved Hide resolved

cli/defradb/cmd/init.go Outdated Show resolved Hide resolved

cli/defradb/cmd/init.go Outdated Show resolved Hide resolved

orpheuslummis force-pushed the orpheus/refactor/config branch from 708bb3f to 4e1c6f0 Compare June 7, 2022 14:46

orpheuslummis mentioned this pull request Jun 7, 2022

Distinguish log output from 'interactive' feedback in CLI #507

Closed

jsimnz approved these changes Jun 19, 2022

View reviewed changes

shahzadlone approved these changes Jun 19, 2022

View reviewed changes

fredcarle approved these changes Jun 20, 2022

View reviewed changes

feat: Introduce top-level config package

ab87614

orpheuslummis force-pushed the orpheus/refactor/config branch from 4e1c6f0 to ab87614 Compare June 28, 2022 16:05

orpheuslummis requested review from AndrewSisley, fredcarle and shahzadlone June 28, 2022 16:15

AndrewSisley approved these changes Jun 28, 2022

View reviewed changes

cli/serverdump.go Outdated Show resolved Hide resolved

config/config.go Show resolved Hide resolved

orpheuslummis added 3 commits June 28, 2022 11:31

use const

56cfeba

integrate feedback regarding documentation

4e348dd

details

f4a7a93

fredcarle approved these changes Jun 29, 2022

View reviewed changes

node/config.go Outdated Show resolved Hide resolved

Following documentation convention as suggested

77de77c

orpheuslummis merged commit b16bf3a into develop Jun 29, 2022

orpheuslummis deleted the orpheus/refactor/config branch June 29, 2022 17:40

This was referenced Jun 29, 2022

CLI: init command #347

Closed

CLI: global flags overhaul #348

Closed

shahzadlone pushed a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024

feat: Introduce top-level config package (sourcenetwork#389)

3520fce

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Introduce top-level config package #389

feat: Introduce top-level config package #389

orpheuslummis commented May 3, 2022 •

edited

Loading

codecov bot commented May 3, 2022 •

edited

Loading

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

orpheuslummis commented May 6, 2022

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

AndrewSisley left a comment

orpheuslummis commented May 24, 2022

orpheuslummis commented May 24, 2022

jsimnz left a comment

shahzadlone left a comment

fredcarle left a comment

orpheuslummis commented Jun 28, 2022

AndrewSisley left a comment

fredcarle left a comment

feat: Introduce top-level config package #389

feat: Introduce top-level config package #389

Conversation

orpheuslummis commented May 3, 2022 • edited Loading

codecov bot commented May 3, 2022 • edited Loading

Codecov Report

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

orpheuslummis commented May 6, 2022

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

AndrewSisley left a comment

Choose a reason for hiding this comment

orpheuslummis commented May 24, 2022

orpheuslummis commented May 24, 2022

jsimnz left a comment

Choose a reason for hiding this comment

shahzadlone left a comment

Choose a reason for hiding this comment

fredcarle left a comment

Choose a reason for hiding this comment

orpheuslummis commented Jun 28, 2022

AndrewSisley left a comment

Choose a reason for hiding this comment

fredcarle left a comment

Choose a reason for hiding this comment

orpheuslummis commented May 3, 2022 •

edited

Loading

codecov bot commented May 3, 2022 •

edited

Loading