Align NewConfig with AddFlags defaults #19254
Conversation
server/embed/config.go
Outdated
| AuthToken: DefaultAuthToken, | ||
| BcryptCost: uint(bcrypt.DefaultCost), | ||
| AuthTokenTTL: 300, | ||
| SelfSignedCertValidity: 1, |
There was a problem hiding this comment.
We need to update the default value in AddFlags: change the 1 with cfg.SelfSignedCertValidity. This comment applies other fields as well.
Line 734 in c9045d6
There was a problem hiding this comment.
I was saying to change
fs.UintVar(&cfg.SelfSignedCertValidity, "self-signed-cert-validity", 1, "The validity period of the client and peer certificates, unit is year")
to
fs.UintVar(&cfg.SelfSignedCertValidity, "self-signed-cert-validity", cfg.SelfSignedCertValidity, "The validity period of the client and peer certificates, unit is year")
Also need to do similar changes for other fields you changed in this PR.
There was a problem hiding this comment.
Oh, sorry I misread your comment. I agree with the direction, however I worried about changing defaults by mistake that is easy to miss. So I proposed to make a minimal step that is easily verifiable by test.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files
... and 29 files with indirect coverage changes @@ Coverage Diff @@
## main #19254 +/- ##
==========================================
+ Coverage 68.79% 68.80% +0.01%
==========================================
Files 420 420
Lines 35649 35669 +20
==========================================
+ Hits 24524 24542 +18
+ Misses 9698 9697 -1
- Partials 1427 1430 +3 Continue to review full report in Codecov by Sentry.
|
Signed-off-by: Marek Siarkowicz <[email protected]>
serathius
force-pushed
the
align-defaults
branch
from
f011488 to
5a556d8
Compare
| ExperimentalTxnModeWriteWithSharedBuffer: ExperimentalTxnModeWriteWithSharedBuffer, | ||
| ExperimentalDistributedTracingAddress: ExperimentalDistributedTracingAddress, | ||
| ExperimentalDistributedTracingServiceName: ExperimentalDistributedTracingServiceName, |
There was a problem hiding this comment.
| ExperimentalTxnModeWriteWithSharedBuffer: ExperimentalTxnModeWriteWithSharedBuffer, | |
| ExperimentalDistributedTracingAddress: ExperimentalDistributedTracingAddress, | |
| ExperimentalDistributedTracingServiceName: ExperimentalDistributedTracingServiceName, | |
| ExperimentalTxnModeWriteWithSharedBuffer: DefaultExperimentalTxnModeWriteWithSharedBuffer, | |
| ExperimentalDistributedTracingAddress: DefaultExperimentalDistributedTracingAddress, | |
| ExperimentalDistributedTracingServiceName: DefaultExperimentalDistributedTracingServiceName, |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ahrtr, serathius The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
cc @ahrtr @fuweid @AwesomePatrol @siyuanfoundation