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.

Sign up for GitHub

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

Jump to bottom

[misc] tidy imports #13885

Merged
merged 1 commit into from
Aug 31, 2023
Merged

[misc] tidy imports #13885

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
21 changes: 11 additions & 10 deletions go/test/endtoend/messaging/message_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,15 @@ import (

"vitess.io/vitess/go/mysql"
"vitess.io/vitess/go/sqltypes"
"vitess.io/vitess/go/vt/log"
"vitess.io/vitess/go/vt/vtgate/vtgateconn"

querypb "vitess.io/vitess/go/vt/proto/query"
topodatapb "vitess.io/vitess/go/vt/proto/topodata"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems strange to break these up into separate blocks if they are all for internal vitess imports?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in most other files we tend to separate imports from the generated protobufs into their own blocks

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We seem very inconsistent about it though? Especially here where the protobuf ones split up multiple vitess import blocks and isn't entirely separate on their own? Anyway, just seems like churn for something we're don't seem to have standardized. And if we should, can we trick something like goimports into enforcing a consistent style? (I don't really even care that much about the style really).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mattlord and i have been trying to enforce it code review a bit more as a "while you're here" thing, so it's generally files that are churning anyway.

https://github.com/incu6us/goimports-reviser looks close, and there's some draft work for supporting named imports as separate groups, but idk if it's worth it 🤷

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't aware that we intentionally try to separate normal vitess imports from generated proto imports. I am actually used to consolidating them.

"vitess.io/vitess/go/test/endtoend/cluster"
"vitess.io/vitess/go/test/endtoend/utils"
cmp "vitess.io/vitess/go/test/utils"
"vitess.io/vitess/go/vt/log"
"vitess.io/vitess/go/vt/proto/query"
querypb "vitess.io/vitess/go/vt/proto/query"
"vitess.io/vitess/go/vt/proto/topodata"
"vitess.io/vitess/go/vt/vtgate/vtgateconn"
)

var testMessage = "{\"message\": \"hello world\"}"
Expand Down Expand Up @@ -509,9 +510,9 @@ func testMessaging(t *testing.T, name, ks string) {
res, err := stream.MessageStream(ks, "", nil, name)
require.Nil(t, err)
require.Equal(t, 3, len(res.Fields))
validateField(t, res.Fields[0], "id", query.Type_INT64)
validateField(t, res.Fields[1], "tenant_id", query.Type_INT64)
validateField(t, res.Fields[2], "message", query.Type_JSON)
validateField(t, res.Fields[0], "id", querypb.Type_INT64)
validateField(t, res.Fields[1], "tenant_id", querypb.Type_INT64)
validateField(t, res.Fields[2], "message", querypb.Type_JSON)

// validate recieved msgs
resMap := make(map[string]string)
Expand Down Expand Up @@ -553,7 +554,7 @@ func testMessaging(t *testing.T, name, ks string) {
assert.Equal(t, uint64(1), qr.RowsAffected)
}

func validateField(t *testing.T, field *query.Field, name string, _type query.Type) {
func validateField(t *testing.T, field *querypb.Field, name string, _type querypb.Type) {
assert.Equal(t, name, field.Name)
assert.Equal(t, _type, field.Type)
}
Expand Down Expand Up @@ -581,7 +582,7 @@ func VtgateGrpcConn(ctx context.Context, cluster *cluster.LocalProcessCluster) (
}

// MessageStream strarts the stream for the corresponding connection.
func (stream *VTGateStream) MessageStream(ks, shard string, keyRange *topodata.KeyRange, name string) (*sqltypes.Result, error) {
func (stream *VTGateStream) MessageStream(ks, shard string, keyRange *topodatapb.KeyRange, name string) (*sqltypes.Result, error) {
// start message stream which send received message to the respChan
session := stream.Session("@primary", nil)
resultStream, err := session.StreamExecute(stream.ctx, fmt.Sprintf("stream * from %s", name), nil)
Expand Down
17 changes: 7 additions & 10 deletions go/vt/vtgate/vcursor_impl_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,18 @@ import (
"strings"
"testing"

querypb "vitess.io/vitess/go/vt/proto/query"
"github.com/stretchr/testify/require"

"vitess.io/vitess/go/vt/proto/vschema"
vschemapb "vitess.io/vitess/go/vt/proto/vschema"
"vitess.io/vitess/go/vt/key"
"vitess.io/vitess/go/vt/sqlparser"
"vitess.io/vitess/go/vt/srvtopo"
"vitess.io/vitess/go/vt/topo"

"vitess.io/vitess/go/vt/key"
"vitess.io/vitess/go/vt/vtgate/vindexes"

"github.com/stretchr/testify/require"

querypb "vitess.io/vitess/go/vt/proto/query"
topodatapb "vitess.io/vitess/go/vt/proto/topodata"
vschemapb "vitess.io/vitess/go/vt/proto/vschema"
vtgatepb "vitess.io/vitess/go/vt/proto/vtgate"
"vitess.io/vitess/go/vt/sqlparser"
)

var _ VSchemaOperator = (*fakeVSchemaOperator)(nil)
Expand All @@ -31,11 +28,11 @@ type fakeVSchemaOperator struct {
vschema *vindexes.VSchema
}

func (f fakeVSchemaOperator) GetCurrentSrvVschema() *vschema.SrvVSchema {
func (f fakeVSchemaOperator) GetCurrentSrvVschema() *vschemapb.SrvVSchema {
panic("implement me")
}

func (f fakeVSchemaOperator) UpdateVSchema(ctx context.Context, ksName string, vschema *vschema.SrvVSchema) error {
func (f fakeVSchemaOperator) UpdateVSchema(ctx context.Context, ksName string, vschema *vschemapb.SrvVSchema) error {
panic("implement me")
}

Expand Down
9 changes: 4 additions & 5 deletions go/vt/vtgate/vindexes/consistent_lookup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,14 @@ import (
"fmt"

"vitess.io/vitess/go/mysql/sqlerror"
"vitess.io/vitess/go/vt/vtgate/evalengine"

"vitess.io/vitess/go/sqltypes"
"vitess.io/vitess/go/vt/key"
"vitess.io/vitess/go/vt/sqlparser"
"vitess.io/vitess/go/vt/vtgate/evalengine"

querypb "vitess.io/vitess/go/vt/proto/query"
topodatapb "vitess.io/vitess/go/vt/proto/topodata"
"vitess.io/vitess/go/vt/proto/vtgate"
vtgatepb "vitess.io/vitess/go/vt/proto/vtgate"
"vitess.io/vitess/go/vt/sqlparser"
)

const (
Expand Down Expand Up @@ -336,7 +335,7 @@ func (lu *clCommon) Verify(ctx context.Context, vcursor VCursor, ids []sqltypes.
}
return out, nil
}
return lu.lkp.VerifyCustom(ctx, vcursor, ids, ksidsToValues(ksids), vtgate.CommitOrder_PRE)
return lu.lkp.VerifyCustom(ctx, vcursor, ids, ksidsToValues(ksids), vtgatepb.CommitOrder_PRE)
}

// Create reserves the id by inserting it into the vindex table.
Expand Down
27 changes: 13 additions & 14 deletions go/vt/vtgate/vstream_manager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,25 +25,24 @@ import (
"testing"
"time"

"vitess.io/vitess/go/test/utils"
"vitess.io/vitess/go/vt/topo"

vtgatepb "vitess.io/vitess/go/vt/proto/vtgate"
vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc"
"vitess.io/vitess/go/vt/vterrors"

"vitess.io/vitess/go/stats"
"vitess.io/vitess/go/vt/vttablet/sandboxconn"
"google.golang.org/protobuf/proto"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"google.golang.org/protobuf/proto"

"vitess.io/vitess/go/stats"
"vitess.io/vitess/go/vt/discovery"
"vitess.io/vitess/go/vt/proto/binlogdata"
"vitess.io/vitess/go/vt/srvtopo"
"vitess.io/vitess/go/vt/topo"
"vitess.io/vitess/go/vt/vterrors"
"vitess.io/vitess/go/vt/vttablet/sandboxconn"

binlogdatapb "vitess.io/vitess/go/vt/proto/binlogdata"
topodatapb "vitess.io/vitess/go/vt/proto/topodata"
"vitess.io/vitess/go/vt/srvtopo"
vtgatepb "vitess.io/vitess/go/vt/proto/vtgate"
vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc"

"vitess.io/vitess/go/test/utils"
)

var mu sync.Mutex
Expand Down Expand Up @@ -749,7 +748,7 @@ func TestVStreamJournalNoMatch(t *testing.T) {
{Type: binlogdatapb.VEventType_GTID, Gtid: "jn1"},
{Type: binlogdatapb.VEventType_COMMIT},
}
wantjn1 := &binlogdata.VStreamResponse{Events: []*binlogdatapb.VEvent{
wantjn1 := &binlogdatapb.VStreamResponse{Events: []*binlogdatapb.VEvent{
{Type: binlogdatapb.VEventType_VGTID, Vgtid: &binlogdatapb.VGtid{
ShardGtids: []*binlogdatapb.ShardGtid{{
Keyspace: ks,
Expand Down Expand Up @@ -797,7 +796,7 @@ func TestVStreamJournalNoMatch(t *testing.T) {
{Type: binlogdatapb.VEventType_GTID, Gtid: "jn2"},
{Type: binlogdatapb.VEventType_COMMIT},
}
wantjn2 := &binlogdata.VStreamResponse{Events: []*binlogdatapb.VEvent{
wantjn2 := &binlogdatapb.VStreamResponse{Events: []*binlogdatapb.VEvent{
{Type: binlogdatapb.VEventType_VGTID, Vgtid: &binlogdatapb.VGtid{
ShardGtids: []*binlogdatapb.ShardGtid{{
Keyspace: ks,
Expand Down
12 changes: 5 additions & 7 deletions go/vt/vtorc/inst/instance_dao.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,19 @@ import (
"github.com/sjmudd/stopwatch"

"vitess.io/vitess/go/mysql/replication"

"vitess.io/vitess/go/stats"
"vitess.io/vitess/go/vt/external/golib/sqlutils"

"vitess.io/vitess/go/tb"
"vitess.io/vitess/go/vt/external/golib/sqlutils"
"vitess.io/vitess/go/vt/log"
replicationdatapb "vitess.io/vitess/go/vt/proto/replicationdata"
topodatapb "vitess.io/vitess/go/vt/proto/topodata"
"vitess.io/vitess/go/vt/topo/topoproto"
"vitess.io/vitess/go/vt/vtorc/collection"
"vitess.io/vitess/go/vt/vtorc/config"
"vitess.io/vitess/go/vt/vtorc/db"
"vitess.io/vitess/go/vt/vtorc/metrics/query"
"vitess.io/vitess/go/vt/vtorc/util"
math "vitess.io/vitess/go/vt/vtorc/util"

replicationdatapb "vitess.io/vitess/go/vt/proto/replicationdata"
topodatapb "vitess.io/vitess/go/vt/proto/topodata"
)

const (
Expand Down Expand Up @@ -557,7 +555,7 @@ func readInstanceRow(m sqlutils.RowMap) *Instance {
instance.Problems = append(instance.Problems, "not_recently_checked")
} else if instance.ReplicationThreadsExist() && !instance.ReplicaRunning() {
instance.Problems = append(instance.Problems, "not_replicating")
} else if instance.ReplicationLagSeconds.Valid && math.AbsInt64(instance.ReplicationLagSeconds.Int64-int64(instance.SQLDelay)) > int64(config.Config.ReasonableReplicationLagSeconds) {
} else if instance.ReplicationLagSeconds.Valid && util.AbsInt64(instance.ReplicationLagSeconds.Int64-int64(instance.SQLDelay)) > int64(config.Config.ReasonableReplicationLagSeconds) {
instance.Problems = append(instance.Problems, "replication_lag")
}
if instance.GtidErrant != "" {
Expand Down
3 changes: 1 addition & 2 deletions go/vt/vttablet/tabletmanager/rpc_vreplication.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ import (
querypb "vitess.io/vitess/go/vt/proto/query"
tabletmanagerdatapb "vitess.io/vitess/go/vt/proto/tabletmanagerdata"
vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc"
topoprotopb "vitess.io/vitess/go/vt/topo/topoproto"
)

const (
Expand Down Expand Up @@ -67,7 +66,7 @@ func (tm *TabletManager) CreateVReplicationWorkflow(ctx context.Context, req *ta
req.Cells = append(req.Cells, tm.Tablet().Alias.Cell)
}
wfState := binlogdatapb.VReplicationWorkflowState_Stopped.String()
tabletTypesStr := topoprotopb.MakeStringTypeCSV(req.TabletTypes)
tabletTypesStr := topoproto.MakeStringTypeCSV(req.TabletTypes)
if req.TabletSelectionPreference == tabletmanagerdatapb.TabletSelectionPreference_INORDER {
tabletTypesStr = discovery.InOrderHint + tabletTypesStr
}
Expand Down
Loading