Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

THRIFT-4448: Golang: do something with context.Context #1459

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion LANGUAGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ The Language/Library Levels indicate the minimum and maximum versions that are u
<tr align=center>
<td align=left><a href="lib/go/README.md">Go</a></td>
<!-- Build Systems ---------><td><img src="doc/images/cgrn.png" alt="Yes"/></td><td><img src="doc/images/cred.png" alt=""/></td>
<!-- Language Levels -------><td>1.2.1</td><td>1.8.3</td>
<!-- Language Levels -------><td>1.7</td><td>1.9.3</td>
<!-- Low-Level Transports --><td><img src="doc/images/cred.png" alt=""/></td><td><img src="doc/images/cred.png" alt=""/></td><td><img src="doc/images/cgrn.png" alt="Yes"/></td><td><img src="doc/images/cred.png" alt=""/></td><td><img src="doc/images/cgrn.png" alt="Yes"/></td><td><img src="doc/images/cgrn.png" alt="Yes"/></td>
<!-- Transport Wrappers ----><td><img src="doc/images/cgrn.png" alt="Yes"/></td><td><img src="doc/images/cgrn.png" alt="Yes"/></td><td><img src="doc/images/cgrn.png" alt="Yes"/></td>
<!-- Protocols -------------><td><img src="doc/images/cgrn.png" alt="Yes"/></td><td><img src="doc/images/cgrn.png" alt="Yes"/></td><td><img src="doc/images/cgrn.png" alt="Yes"/></td><td><img src="doc/images/cgrn.png" alt="Yes"/></td>
Expand Down
2 changes: 1 addition & 1 deletion build/docker/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ Last updated: October 1, 2017
| delphi | | | | Not in CI |
| dotnet | | 2.1.4 | 2.1.4 | v2.1.4 SDK uses v2.0.5 Runtime |
| erlang | R16B03 | 18.3 | 20.0.4 | |
| go | 1.2.1 | 1.6.2 | 1.8.3 | |
| go | 1.7.6 | 1.7.6 | 1.8.3 | |
| haskell | 7.6.3 | 7.10.3 | 8.0.2 | |
| haxe | | 3.2.1 | 3.4.2 | disabled in trusty builds - cores on install v3.0.0, disabled in artful builds - see THRIFT-4352 |
| java | 1.7.0_151 | 1.8.0_151 | 1.8.0_151 | |
Expand Down
12 changes: 9 additions & 3 deletions build/docker/ubuntu-trusty/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,15 @@ RUN apt-get install -y --no-install-recommends \
`# GlibC dependencies` \
libglib2.0-dev

RUN apt-get install -y --no-install-recommends \
`# golang (go) dependencies` \
golang-go
# golang
ENV GOLANG_VERSION 1.7.6
ENV GOLANG_DOWNLOAD_URL https://golang.org/dl/go$GOLANG_VERSION.linux-amd64.tar.gz
ENV GOLANG_DOWNLOAD_SHA256 ad5808bf42b014c22dd7646458f631385003049ded0bb6af2efc7f1f79fa29ea
RUN curl -fsSL "$GOLANG_DOWNLOAD_URL" -o golang.tar.gz && \
echo "$GOLANG_DOWNLOAD_SHA256 golang.tar.gz" | sha256sum -c - && \
tar -C /usr/local -xzf golang.tar.gz && \
ln -s /usr/local/go/bin/go /usr/local/bin && \
rm golang.tar.gz

RUN apt-get install -y --no-install-recommends \
`# Haskell dependencies` \
Expand Down
13 changes: 9 additions & 4 deletions build/docker/ubuntu-xenial/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,15 @@ RUN apt-get install -y --no-install-recommends \
`# GlibC dependencies` \
libglib2.0-dev

RUN apt-get install -y --no-install-recommends \
`# golang (go) dependencies` \
golang-go \
golang-race-detector-runtime
# golang
ENV GOLANG_VERSION 1.7.6
ENV GOLANG_DOWNLOAD_URL https://golang.org/dl/go$GOLANG_VERSION.linux-amd64.tar.gz
ENV GOLANG_DOWNLOAD_SHA256 ad5808bf42b014c22dd7646458f631385003049ded0bb6af2efc7f1f79fa29ea
RUN curl -fsSL "$GOLANG_DOWNLOAD_URL" -o golang.tar.gz && \
echo "$GOLANG_DOWNLOAD_SHA256 golang.tar.gz" | sha256sum -c - && \
tar -C /usr/local -xzf golang.tar.gz && \
ln -s /usr/local/go/bin/go /usr/local/bin && \
rm golang.tar.gz

RUN apt-get install -y --no-install-recommends \
`# Haskell dependencies` \
Expand Down
27 changes: 6 additions & 21 deletions compiler/cpp/src/thrift/generate/t_go_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ class t_go_generator : public t_generator {
gen_package_prefix_ = "";
package_flag = "";
read_write_private_ = false;
legacy_context_ = false;
ignore_initialisms_ = false;
for( iter = parsed_options.begin(); iter != parsed_options.end(); ++iter) {
if( iter->first.compare("package_prefix") == 0) {
Expand All @@ -92,8 +91,6 @@ class t_go_generator : public t_generator {
package_flag = (iter->second);
} else if( iter->first.compare("read_write_private") == 0) {
read_write_private_ = true;
} else if( iter->first.compare("legacy_context") == 0) {
legacy_context_ = true;
} else if( iter->first.compare("ignore_initialisms") == 0) {
ignore_initialisms_ = true;
} else {
Expand Down Expand Up @@ -287,7 +284,6 @@ class t_go_generator : public t_generator {
std::string gen_package_prefix_;
std::string gen_thrift_import_;
bool read_write_private_;
bool legacy_context_;
bool ignore_initialisms_;

/**
Expand Down Expand Up @@ -883,16 +879,10 @@ string t_go_generator::go_imports_begin(bool consts) {
"\t\"database/sql/driver\"\n"
"\t\"errors\"\n";
}
if (legacy_context_) {
extra +=
"\t\"golang.org/x/net/context\"\n";
} else {
extra +=
"\t\"context\"\n";
}
return string(
"import (\n"
"\t\"bytes\"\n"
"\t\"context\"\n"
"\t\"reflect\"\n"
+ extra +
"\t\"fmt\"\n"
Expand Down Expand Up @@ -2073,9 +2063,6 @@ void t_go_generator::generate_service_remote(t_service* tservice) {
string unused_protection;

string ctxPackage = "context";
if (legacy_context_) {
ctxPackage = "golang.org/x/net/context";
}

f_remote << go_autogen_comment();
f_remote << indent() << "package main" << endl << endl;
Expand Down Expand Up @@ -2576,7 +2563,7 @@ void t_go_generator::generate_service_server(t_service* tservice) {
f_types_ << indent() << " oprot.WriteMessageBegin(name, thrift.EXCEPTION, seqId)" << endl;
f_types_ << indent() << " " << x << ".Write(oprot)" << endl;
f_types_ << indent() << " oprot.WriteMessageEnd()" << endl;
f_types_ << indent() << " oprot.Flush()" << endl;
f_types_ << indent() << " oprot.Flush(ctx)" << endl;
f_types_ << indent() << " return false, " << x << endl;
f_types_ << indent() << "" << endl;
f_types_ << indent() << "}" << endl << endl;
Expand Down Expand Up @@ -2641,7 +2628,7 @@ void t_go_generator::generate_process_function(t_service* tservice, t_function*
<< "\", thrift.EXCEPTION, seqId)" << endl;
f_types_ << indent() << " x.Write(oprot)" << endl;
f_types_ << indent() << " oprot.WriteMessageEnd()" << endl;
f_types_ << indent() << " oprot.Flush()" << endl;
f_types_ << indent() << " oprot.Flush(ctx)" << endl;
}
f_types_ << indent() << " return false, err" << endl;
f_types_ << indent() << "}" << endl << endl;
Expand Down Expand Up @@ -2709,7 +2696,7 @@ void t_go_generator::generate_process_function(t_service* tservice, t_function*
<< "\", thrift.EXCEPTION, seqId)" << endl;
f_types_ << indent() << " x.Write(oprot)" << endl;
f_types_ << indent() << " oprot.WriteMessageEnd()" << endl;
f_types_ << indent() << " oprot.Flush()" << endl;
f_types_ << indent() << " oprot.Flush(ctx)" << endl;
}

f_types_ << indent() << " return true, err2" << endl;
Expand Down Expand Up @@ -2746,7 +2733,7 @@ void t_go_generator::generate_process_function(t_service* tservice, t_function*
<< endl;
f_types_ << indent() << " err = err2" << endl;
f_types_ << indent() << "}" << endl;
f_types_ << indent() << "if err2 = oprot.Flush(); err == nil && err2 != nil {" << endl;
f_types_ << indent() << "if err2 = oprot.Flush(ctx); err == nil && err2 != nil {" << endl;
f_types_ << indent() << " err = err2" << endl;
f_types_ << indent() << "}" << endl;
f_types_ << indent() << "if err != nil {" << endl;
Expand Down Expand Up @@ -3642,6 +3629,4 @@ THRIFT_REGISTER_GENERATOR(go, "Go",
" ignore_initialisms\n"
" Disable automatic spelling correction of initialisms (e.g. \"URL\")\n" \
" read_write_private\n"
" Make read/write methods private, default is public Read/Write\n" \
" legacy_context\n"
" Use legacy x/net/context instead of context in go<1.7.\n")
" Make read/write methods private, default is public Read/Write\n")
Empty file.
2 changes: 0 additions & 2 deletions lib/go/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,12 @@ install:
@echo '##############################################################'

check-local:
GOPATH=`pwd` $(GO) get golang.org/x/net/context
GOPATH=`pwd` $(GO) test -race ./thrift

clean-local:
$(RM) -rf pkg

all-local:
GOPATH=`pwd` $(GO) get golang.org/x/net/context
GOPATH=`pwd` $(GO) build ./thrift

EXTRA_DIST = \
Expand Down
2 changes: 2 additions & 0 deletions lib/go/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ under the License.
Using Thrift with Go
====================

Thrift supports Go 1.7+

In following Go conventions, we recommend you use the 'go' tool to install
Thrift for go.

Expand Down
5 changes: 0 additions & 5 deletions lib/go/test/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,6 @@
# under the License.
#

if GOVERSION_LT_17
COMPILER_EXTRAFLAG=",legacy_context"
endif

THRIFTARGS = -out gopath/src/ --gen go:thrift_import=thrift$(COMPILER_EXTRAFLAG)
THRIFTTEST = $(top_srcdir)/test/ThriftTest.thrift

Expand Down Expand Up @@ -59,7 +55,6 @@ gopath: $(THRIFT) $(THRIFTTEST) \
$(THRIFT) $(THRIFTARGS) InitialismsTest.thrift
$(THRIFT) $(THRIFTARGS),read_write_private DontExportRWTest.thrift
$(THRIFT) $(THRIFTARGS),ignore_initialisms IgnoreInitialismsTest.thrift
GOPATH=`pwd`/gopath $(GO) get golang.org/x/net/context
GOPATH=`pwd`/gopath $(GO) get github.com/golang/mock/gomock || true
sed -i 's/\"context\"/\"golang.org\/x\/net\/context\"/g' gopath/src/github.com/golang/mock/gomock/controller.go || true
GOPATH=`pwd`/gopath $(GO) get github.com/golang/mock/gomock
Expand Down
17 changes: 9 additions & 8 deletions lib/go/test/tests/client_error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package tests

import (
"context"
"errors"
"errortest"
"testing"
Expand Down Expand Up @@ -212,7 +213,7 @@ func prepareClientCallReply(protocol *MockTProtocol, failAt int, failWith error)
if failAt == 25 {
err = failWith
}
last = protocol.EXPECT().Flush().Return(err).After(last)
last = protocol.EXPECT().Flush(context.Background()).Return(err).After(last)
if failAt == 25 {
return true
}
Expand Down Expand Up @@ -536,7 +537,7 @@ func prepareClientCallException(protocol *MockTProtocol, failAt int, failWith er
last = protocol.EXPECT().WriteFieldStop().After(last)
last = protocol.EXPECT().WriteStructEnd().After(last)
last = protocol.EXPECT().WriteMessageEnd().After(last)
last = protocol.EXPECT().Flush().After(last)
last = protocol.EXPECT().Flush(context.Background()).After(last)

// Reading the exception, might fail.
if failAt == 0 {
Expand Down Expand Up @@ -704,7 +705,7 @@ func TestClientSeqIdMismatch(t *testing.T) {
protocol.EXPECT().WriteFieldStop(),
protocol.EXPECT().WriteStructEnd(),
protocol.EXPECT().WriteMessageEnd(),
protocol.EXPECT().Flush(),
protocol.EXPECT().Flush(context.Background()),
protocol.EXPECT().ReadMessageBegin().Return("testString", thrift.REPLY, int32(2), nil),
)

Expand Down Expand Up @@ -735,7 +736,7 @@ func TestClientSeqIdMismatchLegeacy(t *testing.T) {
protocol.EXPECT().WriteFieldStop(),
protocol.EXPECT().WriteStructEnd(),
protocol.EXPECT().WriteMessageEnd(),
protocol.EXPECT().Flush(),
protocol.EXPECT().Flush(context.Background()),
protocol.EXPECT().ReadMessageBegin().Return("testString", thrift.REPLY, int32(2), nil),
)

Expand Down Expand Up @@ -764,7 +765,7 @@ func TestClientWrongMethodName(t *testing.T) {
protocol.EXPECT().WriteFieldStop(),
protocol.EXPECT().WriteStructEnd(),
protocol.EXPECT().WriteMessageEnd(),
protocol.EXPECT().Flush(),
protocol.EXPECT().Flush(context.Background()),
protocol.EXPECT().ReadMessageBegin().Return("unknown", thrift.REPLY, int32(1), nil),
)

Expand Down Expand Up @@ -795,7 +796,7 @@ func TestClientWrongMethodNameLegacy(t *testing.T) {
protocol.EXPECT().WriteFieldStop(),
protocol.EXPECT().WriteStructEnd(),
protocol.EXPECT().WriteMessageEnd(),
protocol.EXPECT().Flush(),
protocol.EXPECT().Flush(context.Background()),
protocol.EXPECT().ReadMessageBegin().Return("unknown", thrift.REPLY, int32(1), nil),
)

Expand Down Expand Up @@ -824,7 +825,7 @@ func TestClientWrongMessageType(t *testing.T) {
protocol.EXPECT().WriteFieldStop(),
protocol.EXPECT().WriteStructEnd(),
protocol.EXPECT().WriteMessageEnd(),
protocol.EXPECT().Flush(),
protocol.EXPECT().Flush(context.Background()),
protocol.EXPECT().ReadMessageBegin().Return("testString", thrift.INVALID_TMESSAGE_TYPE, int32(1), nil),
)

Expand Down Expand Up @@ -855,7 +856,7 @@ func TestClientWrongMessageTypeLegacy(t *testing.T) {
protocol.EXPECT().WriteFieldStop(),
protocol.EXPECT().WriteStructEnd(),
protocol.EXPECT().WriteMessageEnd(),
protocol.EXPECT().Flush(),
protocol.EXPECT().Flush(context.Background()),
protocol.EXPECT().ReadMessageBegin().Return("testString", thrift.INVALID_TMESSAGE_TYPE, int32(1), nil),
)

Expand Down
8 changes: 4 additions & 4 deletions tutorial/go/src/go17.go → lib/go/test/tests/context.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// +build go1.7

/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
Expand All @@ -19,8 +17,10 @@
* under the License.
*/

package main
package tests

import "context"
import (
"context"
)

var defaultCtx = context.Background()
47 changes: 0 additions & 47 deletions lib/go/test/tests/go17.go

This file was deleted.

13 changes: 13 additions & 0 deletions lib/go/test/tests/multiplexed_protocol_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package tests

import (
"context"
"multiplexedprotocoltest"
"net"
"testing"
Expand All @@ -36,6 +37,18 @@ func FindAvailableTCPServerPort() net.Addr {
}
}

type FirstImpl struct{}

func (f *FirstImpl) ReturnOne(ctx context.Context) (r int64, err error) {
return 1, nil
}

type SecondImpl struct{}

func (s *SecondImpl) ReturnTwo(ctx context.Context) (r int64, err error) {
return 2, nil
}

func createTransport(addr net.Addr) (thrift.TTransport, error) {
socket := thrift.NewTSocketFromAddrTimeout(addr, TIMEOUT)
transport := thrift.NewTFramedTransport(socket)
Expand Down
8 changes: 8 additions & 0 deletions lib/go/test/tests/one_way_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
package tests

import (
"context"
"fmt"
"net"
"onewaytest"
"testing"
Expand All @@ -36,6 +38,12 @@ func findPort() net.Addr {
}
}

type impl struct{}

func (i *impl) Hi(ctx context.Context, in int64, s string) (err error) { fmt.Println("Hi!"); return }
func (i *impl) Emptyfunc(ctx context.Context) (err error) { return }
func (i *impl) EchoInt(ctx context.Context, param int64) (r int64, err error) { return param, nil }

const TIMEOUT = time.Second

var addr net.Addr
Expand Down
Loading