implement recheck cmd #1837
implement recheck cmd #1837
Conversation
| attachConsole() | ||
| defer detachConsole() |
There was a problem hiding this comment.
don't think this is needed?
There was a problem hiding this comment.
We use the attach/detach command whenever it's a command that a Windows user could be running from the command line, so I think we'd want it here?
| mockKnapsack.On("DesktopUpdateInterval").Return(time.Millisecond * 250) | ||
| mockKnapsack.On("DesktopMenuRefreshInterval").Return(time.Millisecond * 250) | ||
| mockKnapsack.On("KolideServerURL").Return("somewhere-over-the-rainbow.example.com") | ||
| mockKnapsack.On("DesktopEnabled").Return(true) | ||
| mockKnapsack.On("Slogger").Return(slogger) | ||
| mockKnapsack.On("InModernStandby").Return(false) | ||
| mockKnapsack.On("SetDesktopRunnerServerURL", mock.Anything).Return(nil) |
There was a problem hiding this comment.
so much mocking to use the settings writer .... wonder if it's time to create an interface for it that we can pass as a dependency, it would be easier to mock
| @@ -647,3 +647,10 @@ func (fc *FlagController) LocalDevelopmentPath() string { | |||
| WithDefaultString(fc.cmdLineOpts.LocalDevelopmentPath), | |||
| ).get(nil) | |||
| } | |||
|
|
|||
| func (fc *FlagController) SetDesktopRunnerServerURL(url string) error { | |||
There was a problem hiding this comment.
Passing this through a flag feels a little weird to me. Might be right though... Why go this way?
There was a problem hiding this comment.
It does feel weird. I just followed the pattern used to store other data in the sqlite db via the start up settings writer. Perhaps we should create a new table / store in the sqlite db ... runtime_values or something?
There was a problem hiding this comment.
I like runtime_values
| authedMux.Handle(MessageEndpoint, http.HandlerFunc(rs.sendMessage)) | ||
| authMuxHandler := rs.authMiddleware(authedMux) | ||
|
|
||
| unauthedMux.Handle(RecheckEndpoint, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
| attachConsole() | ||
| defer detachConsole() |
There was a problem hiding this comment.
We use the attach/detach command whenever it's a command that a Windows user could be running from the command line, so I think we'd want it here?
| @@ -647,3 +647,10 @@ func (fc *FlagController) LocalDevelopmentPath() string { | |||
| WithDefaultString(fc.cmdLineOpts.LocalDevelopmentPath), | |||
| ).get(nil) | |||
| } | |||
|
|
|||
| func (fc *FlagController) SetDesktopRunnerServerURL(url string) error { | |||
There was a problem hiding this comment.
I like runtime_values
| mockKnapsack.On("DesktopUpdateInterval").Return(time.Millisecond * 250) | ||
| mockKnapsack.On("DesktopMenuRefreshInterval").Return(time.Millisecond * 250) | ||
| mockKnapsack.On("KolideServerURL").Return("somewhere-over-the-rainbow.example.com") | ||
| mockKnapsack.On("DesktopEnabled").Return(true) | ||
| mockKnapsack.On("Slogger").Return(slogger) | ||
| mockKnapsack.On("InModernStandby").Return(false) | ||
| mockKnapsack.On("SetDesktopRunnerServerURL", mock.Anything).Return(nil) |
| authedMux.Handle(MessageEndpoint, http.HandlerFunc(rs.sendMessage)) | ||
| authMuxHandler := rs.authMiddleware(authedMux) | ||
|
|
||
| unauthedMux.Handle(RecheckEndpoint, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
There was a problem hiding this comment.
Would we want to rate limit this endpoint to avoid kicking off a bunch of simultaneous rechecks? Found https://www.alexedwards.net/blog/how-to-rate-limit-http-requests for a fairly quick implementation, but also feeling like it's probably overkill.
No description provided.