Skip to content

Commit

Permalink
Address comments
Browse files Browse the repository at this point in the history
Signed-off-by: heanlan <[email protected]>
  • Loading branch information
heanlan committed Jun 2, 2023
1 parent b3b3623 commit b26c4a0
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 35 deletions.
4 changes: 2 additions & 2 deletions pkg/flowaggregator/clickhouseclient/clickhouseclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -454,10 +454,10 @@ func ConnectClickHouse(config *ClickHouseConfig) (*sql.DB, error) {
func parseDatabaseURL(dbUrl string) (protocol, string, error) {
u, err := url.Parse(dbUrl)
if err != nil {
return protocolUnknown, "", fmt.Errorf("failed to parse ClickHouse database URL: %v", err)
return protocolUnknown, "", fmt.Errorf("failed to parse ClickHouse database URL: %w", err)
}
if u.Path != "" || u.RawQuery != "" || u.User != nil {
return protocolUnknown, "", fmt.Errorf("invalid ClickHouse database URL: '%s': path, query string or user info should not be set", dbUrl)
return protocolUnknown, "", fmt.Errorf("invalid ClickHouse database URL '%s': path, query string or user info should not be set", dbUrl)
}
proto := u.Scheme
switch proto {
Expand Down
56 changes: 23 additions & 33 deletions pkg/flowaggregator/clickhouseclient/clickhouseclient_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -334,56 +334,46 @@ func TestUpdateCH(t *testing.T) {

func TestParseDatabaseURL(t *testing.T) {
testcases := []struct {
url string
expectedProto protocol
expectedAddr string
expectedErr error
url string
expectedProto protocol
expectedAddr string
expectedErrMsg string
}{
{
url: "tcp://127.0.0.1:9000",
expectedProto: protocolTCP,
expectedAddr: "127.0.0.1:9000",
expectedErr: nil,
url: "tcp://127.0.0.1:9000",
expectedProto: protocolTCP,
expectedAddr: "127.0.0.1:9000",
expectedErrMsg: "",
},
{
url: "abc://127.0.0.1:9000",
expectedProto: protocolUnknown,
expectedAddr: "",
expectedErr: fmt.Errorf("connection over abc transport protocol is not supported"),
url: "abc://127.0.0.1:9000",
expectedErrMsg: "connection over abc transport protocol is not supported",
},
{
url: "127.0.0.1:9000",
expectedProto: protocolUnknown,
expectedAddr: "",
expectedErr: fmt.Errorf("failed to parse ClickHouse database URL"),
url: "127.0.0.1:9000",
expectedErrMsg: "failed to parse ClickHouse database URL",
},
{
url: "tcp://user:[email protected]:9000",
expectedProto: protocolUnknown,
expectedAddr: "",
expectedErr: fmt.Errorf("invalid ClickHouse database URL"),
url: "tcp://user:[email protected]:9000",
expectedErrMsg: "invalid ClickHouse database URL",
},
{
url: "tcp://127.0.0.1:9000/path",
expectedProto: protocolUnknown,
expectedAddr: "",
expectedErr: fmt.Errorf("invalid ClickHouse database URL"),
url: "tcp://127.0.0.1:9000/path",
expectedErrMsg: "invalid ClickHouse database URL",
},
{
url: "tcp://127.0.0.1:9000?key=value",
expectedProto: protocolUnknown,
expectedAddr: "",
expectedErr: fmt.Errorf("invalid ClickHouse database URL"),
url: "tcp://127.0.0.1:9000?key=value",
expectedErrMsg: "invalid ClickHouse database URL",
},
}
for _, tc := range testcases {
proto, addr, err := parseDatabaseURL(tc.url)
if tc.expectedErr != nil {
assert.ErrorContains(t, err, tc.expectedErr.Error())
if tc.expectedErrMsg != "" {
assert.Contains(t, err.Error(), tc.expectedErrMsg)
} else {
assert.Nil(t, err)
require.NoError(t, err)
assert.Equal(t, tc.expectedProto, proto)
assert.Equal(t, tc.expectedAddr, addr)
}
assert.Equal(t, tc.expectedProto, proto)
assert.Equal(t, tc.expectedAddr, addr)
}
}

0 comments on commit b26c4a0

Please sign in to comment.