[Breaking] fix(Flags): consolidation and help text improvements #7544
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
karlmcguire
requested review from
manishrjain and
vvbalaji-dgraph
as code owners
added 16 commits
March 10, 2021 17:46
aman-bansal
reviewed
Mar 11, 2021
| @@ -712,8 +717,6 @@ func run() { | |||
| setupCustomTokenizers() | |||
| x.Init() | |||
| x.Config.PortOffset = Alpha.Conf.GetInt("port_offset") | |||
| x.Config.Limit = z.NewSuperFlag(Alpha.Conf.GetString("limit")).MergeAndCheckDefault( | |||
| worker.LimitDefaults) | |||
| x.Config.LimitMutationsNquad = int(x.Config.Limit.GetInt64("mutations-nquad")) | |||
| x.Config.LimitQueryEdge = x.Config.Limit.GetUint64("query-edge") | |||
There was a problem hiding this comment.
just a quick thought wouldnt this be great to actually have struct say
type Limit struct { ... ... }
I am sensing with super flags we are just putting too much reliance over x.SuperFlag.Get* function and causing too much noise in various package configuration. Like limit is super flag but x.Config has all the individual limits options. Seems strange to me and bit unstructured
aaroncarey
added a commit
to dgraph-io/dgraph-docs
that referenced
this pull request
Mar 12, 2021
aman-bansal
added a commit
that referenced
this pull request
Mar 12, 2021
* small SuperFlag --help fixes * update Ristretto * combine telemetry flags * combine telemetry flags * cleanup enable_sentry * minor fixes * get tls working for now * move --mutations flag to --limit superflag * fix limit default * move max_retries to badger superflag * critical path helper * fix * --help fixes * always show default string in --help * clean up cdc flag handling * move cdc defaults outside of _ee * telemetry documentation * cleaning * fix comment * clean up tls defaults * nevermind * removing some extra fields Co-authored-by: aman-bansal <bansalaman2905@gmail.com>
aman-bansal
added a commit
that referenced
this pull request
Mar 12, 2021
… (#7560) * small SuperFlag --help fixes * update Ristretto * combine telemetry flags * combine telemetry flags * cleanup enable_sentry * minor fixes * get tls working for now * move --mutations flag to --limit superflag * fix limit default * move max_retries to badger superflag * critical path helper * fix * --help fixes * always show default string in --help * clean up cdc flag handling * move cdc defaults outside of _ee * telemetry documentation * cleaning * fix comment * clean up tls defaults * nevermind * removing some extra fields Co-authored-by: aman-bansal <bansalaman2905@gmail.com> Co-authored-by: Karl McGuire <karl@karlmcguire.com>
aaroncarey
added a commit
to dgraph-io/dgraph-docs
that referenced
this pull request
Mar 16, 2021
…rt of a CLI Command Reference (#111) * Added flag mapping * Update cli-command-ref.md * terminology update to align with CLI help * Add compound flags per hypermodeinc/dgraph/pull/7544 * Update cli-command-ref.md * Update cli-command-ref.md * Add basic information for each command * Old flags formatted as flags * Complete cli-command-ref.md * spellcheck etc. * Update cli-command-ref.md
aaroncarey
added a commit
to dgraph-io/dgraph-docs
that referenced
this pull request
Mar 22, 2021
…s replaced by superflags and their options, plus spellcheck and some minor edits (#122) * Added flag mapping * Update cli-command-ref.md * terminology update to align with CLI help * Add compound flags per hypermodeinc/dgraph/pull/7544 * Update cli-command-ref.md * Update cli-command-ref.md * Add basic information for each command * Old flags formatted as flags * Complete cli-command-ref.md * Updates per superflags Updates thru --enable_sentry, next is --tls_* * spellcheck * Update cli-command-ref.md * Adding details to command list table * Update cli-command-ref.md * Update content/deploy/dgraph-administration.md Co-authored-by: Abu Sakib <sakib@dgraph.io> * Apply suggestions from code review Co-authored-by: Abu Sakib <sakib@dgraph.io> * Apply suggestions from code review Co-authored-by: Abu Sakib <sakib@dgraph.io> * Delete cli-command-ref.md.bak * Update tls-configuration.md * Apply suggestions from code review Co-authored-by: Abu Sakib <sakib@dgraph.io> * superflag replacements (final self-review next) * feedback incorporation / pre-review fixes * Final self-review, spellcheck using `aspell check -M -x file-name.md` * final tweaks * Update content/deploy/config.md Co-authored-by: Abu Sakib <sakib@dgraph.io> * Update content/deploy/kubernetes.md Co-authored-by: Abu Sakib <sakib@dgraph.io> * Update content/deploy/kubernetes.md Co-authored-by: Abu Sakib <sakib@dgraph.io> * Update content/deploy/single-host-setup.md Co-authored-by: Abu Sakib <sakib@dgraph.io> * Update content/deploy/single-host-setup.md Co-authored-by: Abu Sakib <sakib@dgraph.io> * Update content/deploy/single-host-setup.md Co-authored-by: Abu Sakib <sakib@dgraph.io> * Update content/deploy/single-host-setup.md Co-authored-by: Abu Sakib <sakib@dgraph.io> * Feedback incorporation Co-authored-by: Abu Sakib <sakib@dgraph.io>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
DGRAPH-3172
--telemetrytelemetryreportsenable_sentrysentry--limitmutationsmutations--badgermax_retriesmax-retriesThese are the only flag names changed. They've all been moved into their respective SuperFlags, and only
--telemetryis new.Note: In DGRAPH-3172 it says to move
max_retriesinto--badger"only after verifying if that flag is used only in commitToDisk." I made sure the only time the flag is used is here in thetoDiskfunction, but I'm not sure if that's exactly what the ticket is referring to.This change is