HTTP communication #174
Open
HTTP communication #174
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
vqhuy
reviewed
Apr 24, 2017
keyserver/server.go
Outdated
| // Shutdown closes all of the server's connections and shuts down the server. | ||
| func (server *ConiksServer) Shutdown() error { | ||
| close(server.stop) | ||
| if server.httpServer != nil { | ||
| ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) |
There was a problem hiding this comment.
Can we move this scope to line 199 (https://github.com/coast-team/coniks-go/blob/92c82f41423983acc2e0d2e60fb0858527b5e6e2/keyserver/server.go#L199) as
go func() {
<-server.stop
// shutdown httpServer [...]
}()
and remove httpServer from ConiksServer?
There was a problem hiding this comment.
Yes, it is definitely a better solution !
vqhuy
reviewed
Apr 26, 2017
keyserver/server.go
Outdated
| go func() { | ||
| httpSrv.Serve(ln) | ||
| }() | ||
| go func() { |
There was a problem hiding this comment.
Please add server.waitStop.Add(1) before this routine and server.waitStop.Done() after httpSrv.Shutdown().
* Part of coniks-sys#151 * Add client-auditor messages * Create audit log structure, query API finished * Add/Update docs to include auditor, add ReqUnknownDirectory auditor error * Use single generic verifySTRConsistency to be used by client and auditor * Add tests for audit log, debug audit log * Add assertions to validate auditor messages on client * Add generic STR response handler * # This is a combination of 2 commits. # The first commit's message is: Add TODO to move all generic STR auditing code to a separate module # The 2nd commit message will be skipped: # Use single generic verifySTRConsistency to be used by client and auditor * Fix documentation * Use DirSTR instead of merkletree.SignedTreeRoot in auditlog * Remove all references to auditor-directory communication, make auditor response message generic * STRList -> STRHistoryRange * Fail sooner in GetObservedSTRs * Revert changes to protocol/str.go * Always request epoch range in AuditingRequest, fix Insert() bug * Index audit log by hash of directory's initial STR * Insert latest STR into snapshot map right away * Fix go vet error * Change audit log index from byte array to string * Add test case for getting an STR after an audit log update * Refactor common functions * Use slice for initial history inserted, remove IsKnownDirectory * dirName --> addr, some documentation fixes * Small fixes * Revert "Change audit log index from byte array to string" This reverts commit 7e54087. Conflicts: protocol/auditlog.go * More fixes for reverting
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.
Hello,
the HTTP communication feature discussed in #165 has been added.
For now we can still use TCP or Unix sockets, it depends on the config file (replace
tcp://127.0.0.1:3000byhttps://127.0.0.1:3000for example)I hope this is OK but do not hesitate to tell me if there is something wrong with the code
EDIT : Oh yes I forgot to say that this needs the 1.8 version of Go. Prior to this they were no shutdown function for an HTTP server.