added support for max_bytes when creating queues #122
Conversation
Signed-off-by: Daan Gerits <[email protected]>
Signed-off-by: Daan Gerits <[email protected]>
There was a problem hiding this comment.
Mostly good, lets fix the spacing and also render the value in
Line 206 in cc8b297
ajc/queue_command.go
Outdated
| "github.com/dustin/go-humanize" | ||
| "github.com/choria-io/asyncjobs" | ||
| "github.com/choria-io/fisk" | ||
| "github.com/dustin/go-humanize" |
There was a problem hiding this comment.
bad things happened with formatting :)
There was a problem hiding this comment.
yeah, seems my indenting was wrong, sorry about that. Should be fixed in the next commit
Signed-off-by: Daan Gerits <[email protected]>
storage.go
Outdated
|
|
||
| opts = append(opts, jsm.MaxAge(retention)) | ||
| opts = append(opts, jsm.MaxAge(retention)) | ||
| opts = append(opts, jsm.MaxBytes(DefaultMaxBytes)) |
There was a problem hiding this comment.
Not sure I am comfortable adding this maximum always, wrap it in a check for either being set specifically or if its required, check for required using this. To detrmine if its set you can use the IsSetByUser() support when adding the flag
https://github.com/nats-io/jsm.go/blob/78b71b6a1d19e714732a57a7d130bb697b7ce522/manager.go#L94
There was a problem hiding this comment.
Example use here:
f.Flag("subscriptions", "Maximum allowed subscriptions").Default("-1").IsSetByUser(&c.maxSubIsSet).Int64Var(&c.maxSubs)
this will set the boolean true if the user set it
also fixed the formatting of the code and introduced a new MaxBytes option to the client Signed-off-by: Daan Gerits <[email protected]>
|
Best way to avoid indent issues is to use tabs (not spaces for go) and to run go fmt over the files you edit, go fmt takes out all the guess work :) |
Signed-off-by: Daan Gerits <[email protected]>
Good point. Should be ok now |
|
Still several files with unwanted whitespace changes in the PR |
Signed-off-by: Daan Gerits <[email protected]>
|
Ok, that should do it. Sorry man. I owe you a beer at some point ;) |
|
looking better, I'll take a look in a week or so as I am OOO atm. |
|
Meanwhile I see one test is failing to compile: |
Signed-off-by: Daan Gerits <[email protected]>
| @@ -62,6 +64,7 @@ func newDefaultQueue() *Queue { | |||
| MaxRunTime: time.Minute, | |||
| MaxTries: 100, | |||
| MaxConcurrent: DefaultQueueMaxConcurrent, | |||
| MaxBytes: DefaultMaxBytes, | |||
There was a problem hiding this comment.
We should only set this when set by the user, not by default.
| }) | ||
| } | ||
|
|
||
| if maxBytesSet { |
There was a problem hiding this comment.
there is a mix of approaches here, in one place you check for > -1 (line 758) and here you check if its set. I dont think the isSet here really add value
I think now that we never rely on default down here etc we can probably just act on > -1 as you did above. Eitherway we should be consistent whichever we pick
When working against Synadia Cloud, creating a stream requires
max_bytesto be provided. This PR adds that to the CLI.