Merge pull request #577 from uber/dev

kriskowal · web-flow · commit 79387824978f · 2017-02-01T17:34:48.000-08:00
Version 1.3.0
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,11 @@
 Changelog
 =========
 
+# v1.3.0
+
+* Exposes the channel's RootPeerList with `channel.RootPeers()`.
+* Support Thrift namespaces for thrift-gen.
+
 # v1.2.3
 
 * Improve error messages when an argument reader is closed without
@@ -9,7 +14,7 @@ Changelog
   but none was found (e.g., exception is from the future). (#566)
 * Fix ListenIP selecting docker interfaces over physical networks. (#565)
 * Fix for error when a Thrift payload has completed decoding and attempts
-  to close the argument reader without waiting till EOF.  (#564)
+  to close the argument reader without waiting until EOF.  (#564)
 * thrift-gen: Fix "namespace go" being ignored even though the Apache thrift
   generated code was respecting it. (#559)
 
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -5,14 +5,13 @@ We'd love your help making tchannel-go great!
 
 ## Getting Started
 
-TChannel uses [godep](https://github.com/tools/godep) to manage dependencies.
+TChannel uses [glide](https://github.com/Masterminds/glide) to manage
+dependencies.
 To get started:
 
 ```bash
 go get github.com/uber/tchannel-go
-go get github.com/tools/godep
-cd $GOPATH/src/github.com/uber/tchannel-go
-godep restore
+make install_glide
 make  # tests should pass
 ```
 
@@ -32,3 +31,14 @@ pull request is most likely to be accepted if it:
   review comments](https://github.com/golang/go/wiki/CodeReviewComments).
 * Has a [good commit
   message](http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html).
+
+## Cutting a Release
+
+* Send a pull request against dev including:
+  * update CHANGELOG.md (`scripts/changelog_halp.sh`)
+  * update version.go
+* Send a pull request for dev into master
+* `git tag -m v0.0.0 -a v0.0.0`
+* `git push origin --tags`
+* Copy CHANGELOG.md fragment into release notes on
+  https://github.com/uber/tchannel-go/releases 
diff --git a/Makefile b/Makefile
@@ -66,7 +66,9 @@ else
 endif
 
 install_glide:
-	GOPATH=$(OLD_GOPATH) go get -u github.com/Masterminds/glide
+	# all we want is: GOPATH=$(OLD_GOPATH) go get -u github.com/Masterminds/glide
+	# but have to pin to 0.12.3 due to https://github.com/Masterminds/glide/issues/745
+	GOPATH=$(OLD_GOPATH) go get -u github.com/Masterminds/glide && cd $(OLD_GOPATH)/src/github.com/Masterminds/glide && git checkout v0.12.3 && go install
 
 install_ci: install_glide install_lint get_thrift install
 	GOPATH=$(OLD_GOPATH) go get -u github.com/mattn/goveralls
diff --git a/channel.go b/channel.go
@@ -55,6 +55,9 @@ type ChannelOptions struct {
 	// The name of the process, for logging and reporting to peers
 	ProcessName string
 
+	// OnPeerStatusChanged
+	OnPeerStatusChanged func(*Peer)
+
 	// The logger to use for this channel
 	Logger Logger
 
@@ -363,18 +366,18 @@ func (ch *Channel) Peers() *PeerList {
 	return ch.peers
 }
 
-// rootPeers returns the root PeerList for the channel, which is the sole place
+// RootPeers returns the root PeerList for the channel, which is the sole place
 // new Peers are created. All children of the root list (including ch.Peers())
 // automatically re-use peers from the root list and create new peers in the
 // root list.
-func (ch *Channel) rootPeers() *RootPeerList {
+func (ch *Channel) RootPeers() *RootPeerList {
 	return ch.peers.parent
 }
 
 // BeginCall starts a new call to a remote peer, returning an OutboundCall that can
 // be used to write the arguments of the call.
 func (ch *Channel) BeginCall(ctx context.Context, hostPort, serviceName, methodName string, callOptions *CallOptions) (*OutboundCall, error) {
-	p := ch.rootPeers().GetOrAdd(hostPort)
+	p := ch.RootPeers().GetOrAdd(hostPort)
 	return p.BeginCall(ctx, serviceName, methodName, callOptions)
 }
 
@@ -431,7 +434,7 @@ func (ch *Channel) serve() {
 
 // Ping sends a ping message to the given hostPort and waits for a response.
 func (ch *Channel) Ping(ctx context.Context, hostPort string) error {
-	peer := ch.rootPeers().GetOrAdd(hostPort)
+	peer := ch.RootPeers().GetOrAdd(hostPort)
 	conn, err := peer.GetConnection(ctx)
 	if err != nil {
 		return err
@@ -524,7 +527,7 @@ func (ch *Channel) exchangeUpdated(c *Connection) {
 		return
 	}
 
-	p, ok := ch.rootPeers().Get(c.remotePeerInfo.HostPort)
+	p, ok := ch.RootPeers().Get(c.remotePeerInfo.HostPort)
 	if !ok {
 		return
 	}
@@ -575,7 +578,7 @@ func (ch *Channel) connectionActive(c *Connection, direction connectionDirection
 }
 
 func (ch *Channel) addConnectionToPeer(hostPort string, c *Connection, direction connectionDirection) {
-	p := ch.rootPeers().GetOrAdd(hostPort)
+	p := ch.RootPeers().GetOrAdd(hostPort)
 	if err := p.addConnection(c, direction); err != nil {
 		c.log.WithFields(
 			LogField{"remoteHostPort", c.remotePeerInfo.HostPort},
@@ -620,13 +623,13 @@ func (ch *Channel) getMinConnectionState() connectionState {
 // connectionCloseStateChange is called when a connection's close state changes.
 func (ch *Channel) connectionCloseStateChange(c *Connection) {
 	ch.removeClosedConn(c)
-	if peer, ok := ch.rootPeers().Get(c.remotePeerInfo.HostPort); ok {
+	if peer, ok := ch.RootPeers().Get(c.remotePeerInfo.HostPort); ok {
 		peer.connectionCloseStateChange(c)
 		ch.updatePeer(peer)
 	}
 	if c.outboundHP != "" && c.outboundHP != c.remotePeerInfo.HostPort {
 		// Outbound connections may be in multiple peers.
-		if peer, ok := ch.rootPeers().Get(c.outboundHP); ok {
+		if peer, ok := ch.RootPeers().Get(c.outboundHP); ok {
 			peer.connectionCloseStateChange(c)
 			ch.updatePeer(peer)
 		}
diff --git a/introspection.go b/introspection.go
@@ -216,7 +216,7 @@ func (ch *Channel) IntrospectState(opts *IntrospectionOptions) *RuntimeState {
 		CreatedStack:   ch.createdStack,
 		LocalPeer:      ch.PeerInfo(),
 		SubChannels:    ch.subChannels.IntrospectState(opts),
-		RootPeers:      ch.rootPeers().IntrospectState(opts),
+		RootPeers:      ch.RootPeers().IntrospectState(opts),
 		Peers:          ch.Peers().IntrospectList(opts),
 		NumConnections: numConns,
 		Connections:    connIDs,
diff --git a/relay.go b/relay.go
@@ -196,7 +196,7 @@ func NewRelayer(ch *Channel, conn *Connection) *Relayer {
 		localHandler: ch.relayLocal,
 		outbound:     newRelayItems(ch.Logger().WithFields(LogField{"relay", "outbound"})),
 		inbound:      newRelayItems(ch.Logger().WithFields(LogField{"relay", "inbound"})),
-		peers:        ch.rootPeers(),
+		peers:        ch.RootPeers(),
 		conn:         conn,
 		logger:       conn.log,
 	}
diff --git a/testutils/data.go b/testutils/data.go
@@ -37,8 +37,8 @@ var (
 
 func checkCacheSize(n int) {
 	// Start with a reasonably large cache.
-	if n < 8 {
-		n = 8
+	if n < 1024 {
+		n = 1024
 	}
 
 	randMut.RLock()
diff --git a/thrift/thrift-gen/compile_test.go b/thrift/thrift-gen/compile_test.go
@@ -50,8 +50,8 @@ var (
 func TestMain(m *testing.M) {
 	exitCode := m.Run()
 
-	// If we created a fake GOPATH, we should clean it up.
-	if _testGoPath != "" {
+	// If we created a fake GOPATH, we should clean it up on success.
+	if _testGoPath != "" && exitCode == 0 {
 		os.RemoveAll(_testGoPath)
 	}
 
@@ -193,7 +193,7 @@ func TestExternalTemplate(t *testing.T) {
 		}
 
 		// Verify the contents of the extra file.
-		outFile := filepath.Join(dir, packageName(templateFile)+"-service_extend.go")
+		outFile := filepath.Join(dir, defaultPackageName(templateFile)+"-service_extend.go")
 		return verifyFileContents(outFile, expected)
 	}
 	if err := runTest(t, opts, checks); err != nil {
@@ -301,7 +301,7 @@ func checkDirectoryFiles(dir string, n int) error {
 
 func runBuildTest(t *testing.T, thriftFile string) error {
 	extraChecks := func(dir string) error {
-		return checkDirectoryFiles(filepath.Join(dir, packageName(thriftFile)), 4)
+		return checkDirectoryFiles(filepath.Join(dir, defaultPackageName(thriftFile)), 4)
 	}
 
 	opts := processOptions{InputFile: thriftFile}
diff --git a/thrift/thrift-gen/extends.go b/thrift/thrift-gen/extends.go
@@ -45,6 +45,7 @@ func setExtends(state map[string]parseState) error {
 				searchFor = s.Extends
 			} else {
 				include := v.global.includes[parts[0]]
+				s.ExtendsPrefix = include.pkg + "."
 				searchServices = state[include.file].services
 				searchFor = parts[1]
 			}
diff --git a/thrift/thrift-gen/generate.go b/thrift/thrift-gen/generate.go
@@ -71,7 +71,7 @@ func runThrift(inFile string, outDir string) error {
 	}
 
 	// Delete any existing generated code for this Thrift file.
-	genDir := filepath.Join(outDir, packageName(inFile))
+	genDir := filepath.Join(outDir, defaultPackageName(inFile))
 	if err := execCmd("rm", "-rf", genDir); err != nil {
 		return fmt.Errorf("failed to delete directory %s: %v", genDir, err)
 	}
diff --git a/thrift/thrift-gen/main.go b/thrift/thrift-gen/main.go
@@ -115,7 +115,7 @@ func processFile(opts processOptions) error {
 	}
 
 	for filename, v := range allParsed {
-		pkg := packageName(filename)
+		pkg := getNamespace(filename, v.ast)
 
 		for _, template := range allTemplates {
 			outputFile := filepath.Join(opts.OutputDir, pkg, template.outputFile(pkg))
@@ -180,14 +180,19 @@ func parseFile(inputFile string) (map[string]parseState, error) {
 	return allParsed, setExtends(allParsed)
 }
 
+func defaultPackageName(fullPath string) string {
+	filename := filepath.Base(fullPath)
+	file := strings.TrimSuffix(filename, filepath.Ext(filename))
+	return strings.ToLower(file)
+}
+
 func getNamespace(filename string, v *parser.Thrift) string {
 	if ns, ok := v.Namespaces["go"]; ok {
 		return ns
 	}
 
-	base := filepath.Base(filename)
-	base = strings.TrimSuffix(base, filepath.Ext(base))
-	return strings.ToLower(base)
+	// TODO(prashant): Remove any characters that are not valid in Go package names.
+	return defaultPackageName(filename)
 }
 
 func generateCode(outputFile string, template *Template, pkg string, state parseState) error {
@@ -212,13 +217,6 @@ func generateCode(outputFile string, template *Template, pkg string, state parse
 	return template.execute(outputFile, td)
 }
 
-func packageName(fullPath string) string {
-	// TODO(prashant): Remove any characters that are not valid in Go package names.
-	_, filename := filepath.Split(fullPath)
-	file := strings.TrimSuffix(filename, filepath.Ext(filename))
-	return strings.ToLower(file)
-}
-
 type stringSliceFlag []string
 
 func (s *stringSliceFlag) String() string {
diff --git a/thrift/thrift-gen/template.go b/thrift/thrift-gen/template.go
@@ -71,7 +71,7 @@ func parseTemplateFile(file string) (*Template, error) {
 		return nil, fmt.Errorf("failed to parse template in file %q: %v", file, err)
 	}
 
-	return &Template{packageName(file), t}, nil
+	return &Template{defaultPackageName(file), t}, nil
 }
 
 func contextType() string {
diff --git a/thrift/thrift-gen/test_files/include_test/namespace/a/shared.thrift b/thrift/thrift-gen/test_files/include_test/namespace/a/shared.thrift
@@ -3,3 +3,7 @@ namespace go a_shared
 include "../b/shared.thrift"
 
 typedef shared.b_string a_string
+
+service AShared extends shared.BShared {
+	bool healthA()
+}
diff --git a/thrift/thrift-gen/test_files/include_test/namespace/b/shared.thrift b/thrift/thrift-gen/test_files/include_test/namespace/b/shared.thrift
@@ -1,3 +1,7 @@
 namespace go b_shared
 
 typedef string b_string
+
+service BShared {
+	bool healthB()
+}
diff --git a/thrift/thrift-gen/test_files/include_test/namespace/namespace.thrift b/thrift/thrift-gen/test_files/include_test/namespace/namespace.thrift
@@ -1,5 +1,5 @@
 include "a/shared.thrift"
 
-service Foo {
+service Foo extends shared.AShared {
  void Foo(1: shared.a_string str)
 }
diff --git a/thrift/thrift-gen/wrap.go b/thrift/thrift-gen/wrap.go
@@ -37,8 +37,9 @@ type Service struct {
 	*parser.Service
 	state *State
 
-	// ExtendsService is not set in wrap, but in setExtends.
+	// ExtendsService and ExtendsPrefix are set in `setExtends`.
 	ExtendsService *Service
+	ExtendsPrefix  string
 
 	// methods is a cache of all methods.
 	methods []*Method
@@ -91,7 +92,7 @@ func (s *Service) HasExtends() bool {
 // ExtendsServicePrefix returns a package selector (if any) for the extended service.
 func (s *Service) ExtendsServicePrefix() string {
 	if dotIndex := strings.Index(s.Extends, "."); dotIndex > 0 {
-		return s.Extends[:dotIndex+1]
+		return s.ExtendsPrefix
 	}
 	return ""
 }
diff --git a/utils_for_test.go b/utils_for_test.go
@@ -33,11 +33,6 @@ import (
 // MexChannelBufferSize is the size of the message exchange channel buffer.
 const MexChannelBufferSize = mexChannelBufferSize
 
-// RootPeers returns the root peer list from the Channel.
-func (ch *Channel) RootPeers() *RootPeerList {
-	return ch.rootPeers()
-}
-
 // SetOnUpdate sets onUpdate for a peer, which is called when the peer's score is
 // updated in all peer lists.
 func (p *Peer) SetOnUpdate(f func(*Peer)) {
diff --git a/version.go b/version.go
@@ -23,4 +23,4 @@ package tchannel
 // VersionInfo identifies the version of the TChannel library.
 // Due to lack of proper package management, this version string will
 // be maintained manually.
-const VersionInfo = "1.2.3"
+const VersionInfo = "1.3.0"

Original file line number	Diff line number	Diff line change
`@@ -196,7 +196,7 @@ func NewRelayer(ch Channel, conn Connection) *Relayer {`
`196`	`196`	`localHandler: ch.relayLocal,`
`197`	`197`	`outbound: newRelayItems(ch.Logger().WithFields(LogField{"relay", "outbound"})),`
`198`	`198`	`inbound: newRelayItems(ch.Logger().WithFields(LogField{"relay", "inbound"})),`
`199`		`- peers: ch.rootPeers(),`
	`199`	`+ peers: ch.RootPeers(),`
`200`	`200`	`conn: conn,`
`201`	`201`	`logger: conn.log,`
`202`	`202`	`}`
Original file line number	Diff line number	Diff line change
`@@ -37,8 +37,8 @@ var (`
`37`	`37`
`38`	`38`	`func checkCacheSize(n int) {`
`39`	`39`	`// Start with a reasonably large cache.`
`40`		`- if n < 8 {`
`41`		`- n = 8`
	`40`	`+ if n < 1024 {`
	`41`	`+ n = 1024`
`42`	`42`	`}`
`43`	`43`
`44`	`44`	`randMut.RLock()`
Original file line number	Diff line number	Diff line change
`@@ -50,8 +50,8 @@ var (`
`50`	`50`	`func TestMain(m *testing.M) {`
`51`	`51`	`exitCode := m.Run()`
`52`	`52`
`53`		`- // If we created a fake GOPATH, we should clean it up.`
`54`		`- if _testGoPath != "" {`
	`53`	`+ // If we created a fake GOPATH, we should clean it up on success.`
	`54`	`+ if _testGoPath != "" && exitCode == 0 {`
`55`	`55`	`os.RemoveAll(_testGoPath)`
`56`	`56`	`}`
`57`	`57`
`@@ -193,7 +193,7 @@ func TestExternalTemplate(t *testing.T) {`
`193`	`193`	`}`
`194`	`194`
`195`	`195`	`// Verify the contents of the extra file.`
`196`		`- outFile := filepath.Join(dir, packageName(templateFile)+"-service_extend.go")`
	`196`	`+ outFile := filepath.Join(dir, defaultPackageName(templateFile)+"-service_extend.go")`
`197`	`197`	`return verifyFileContents(outFile, expected)`
`198`	`198`	`}`
`199`	`199`	`if err := runTest(t, opts, checks); err != nil {`
`@@ -301,7 +301,7 @@ func checkDirectoryFiles(dir string, n int) error {`
`301`	`301`
`302`	`302`	`func runBuildTest(t *testing.T, thriftFile string) error {`
`303`	`303`	`extraChecks := func(dir string) error {`
`304`		`- return checkDirectoryFiles(filepath.Join(dir, packageName(thriftFile)), 4)`
	`304`	`+ return checkDirectoryFiles(filepath.Join(dir, defaultPackageName(thriftFile)), 4)`
`305`	`305`	`}`
`306`	`306`
`307`	`307`	`opts := processOptions{InputFile: thriftFile}`
Original file line number	Diff line number	Diff line change
`@@ -45,6 +45,7 @@ func setExtends(state map[string]parseState) error {`
`45`	`45`	`searchFor = s.Extends`
`46`	`46`	`} else {`
`47`	`47`	`include := v.global.includes[parts[0]]`
	`48`	`+ s.ExtendsPrefix = include.pkg + "."`
`48`	`49`	`searchServices = state[include.file].services`
`49`	`50`	`searchFor = parts[1]`
`50`	`51`	`}`
Original file line number	Diff line number	Diff line change
`@@ -71,7 +71,7 @@ func runThrift(inFile string, outDir string) error {`
`71`	`71`	`}`
`72`	`72`
`73`	`73`	`// Delete any existing generated code for this Thrift file.`
`74`		`- genDir := filepath.Join(outDir, packageName(inFile))`
	`74`	`+ genDir := filepath.Join(outDir, defaultPackageName(inFile))`
`75`	`75`	`if err := execCmd("rm", "-rf", genDir); err != nil {`
`76`	`76`	`return fmt.Errorf("failed to delete directory %s: %v", genDir, err)`
`77`	`77`	`}`