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

Check minor version and default keys to match in compatibility matrix #1063

Merged
merged 10 commits into from
Feb 3, 2023
14 changes: 14 additions & 0 deletions airflow/docker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,9 @@ services:
ports:
- 127.0.0.1:5432:5432
volumes:

- postgres_data:/var/lib/postgresql/data

environment:
POSTGRES_USER: postgres
POSTGRES_PASSWORD: postgres
Expand All @@ -115,7 +117,9 @@ services:
- airflow_home/include:/usr/local/airflow/include:z
- airflow_home/tests:/usr/local/airflow/tests:z


- airflow_logs:/usr/local/airflow/logs



webserver:
Expand Down Expand Up @@ -146,7 +150,9 @@ services:
- airflow_home/plugins:/usr/local/airflow/plugins:z
- airflow_home/include:/usr/local/airflow/include:z
- airflow_home/tests:/usr/local/airflow/tests:z

- airflow_logs:/usr/local/airflow/logs

healthcheck:
test: curl --fail http://webserver:8080/health || exit 1
interval: 2s
Expand Down Expand Up @@ -200,7 +206,9 @@ services:
ports:
- 127.0.0.1:5432:5432
volumes:

- postgres_data:/var/lib/postgresql/data

environment:
POSTGRES_USER: postgres
POSTGRES_PASSWORD: postgres
Expand All @@ -226,7 +234,9 @@ services:
- airflow_home/include:/usr/local/airflow/include:z
- airflow_home/tests:/usr/local/airflow/tests:z


- airflow_logs:/usr/local/airflow/logs



webserver:
Expand Down Expand Up @@ -257,7 +267,9 @@ services:
- airflow_home/plugins:/usr/local/airflow/plugins:z
- airflow_home/include:/usr/local/airflow/include:z
- airflow_home/tests:/usr/local/airflow/tests:z

- airflow_logs:/usr/local/airflow/logs

healthcheck:
test: curl --fail http://webserver:8080/health || exit 1
interval: 2s
Expand Down Expand Up @@ -285,7 +297,9 @@ services:
- airflow_home/dags:/usr/local/airflow/dags:z
- airflow_home/plugins:/usr/local/airflow/plugins:z
- airflow_home/include:/usr/local/airflow/include:z

- airflow_logs:/usr/local/airflow/logs



`
Expand Down
12 changes: 0 additions & 12 deletions cmd/sql/flow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,24 +172,12 @@ func execFlowCmd(args ...string) error {
return err
}

func execFlowCmdWrongVersion(args ...string) error {
version.CurrVersion = "foo"
cmd := NewFlowCommand()
cmd.SetArgs(args)
_, err := cmd.ExecuteC()
return err
}

func TestFlowCmd(t *testing.T) {
defer patchExecuteCmdInDocker(t, 0, nil)()
err := execFlowCmd()
assert.NoError(t, err)
}

func TestFlowCmdWrongVersion(t *testing.T) {
assert.PanicsWithError(t, "error running []: error parsing response for SQL CLI version %!w(<nil>)", func() { execFlowCmdWrongVersion() })
}

func TestFlowCmdError(t *testing.T) {
defer patchExecuteCmdInDocker(t, 0, errMock)()
err := execFlowCmd("version")
Expand Down
1 change: 1 addition & 0 deletions pkg/testing/testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ func NewTestConfig(platform string) []byte {
local:
enabled: true
host: http://localhost:8871/v1
duplicate_volumes: true
context: %s
contexts:
%s:
Expand Down
17 changes: 16 additions & 1 deletion sql/flow_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/json"
"fmt"
"net/http"
"regexp"
)

type configResponse struct {
Expand Down Expand Up @@ -38,6 +39,13 @@ var (

var httpClient = &http.Client{}

var semVerRegex = regexp.MustCompile("^(.*)[.](.*)[.](.*)$")
pankajkoti marked this conversation as resolved.
Show resolved Hide resolved

const (
trimmedMinorVersionRegexMatchString = "$1.$2"
defaultVersionString = "default"
)

type AstroSQLCliVersion struct {
Version string
Prerelease bool
Expand All @@ -61,7 +69,14 @@ func GetPypiVersion(configURL, astroCliVersion string) (AstroSQLCliVersion, erro
}
SQLCliCompatibility, exists := resp.SQLCliCompatibility[astroCliVersion]
if !exists {
return AstroSQLCliVersion{}, fmt.Errorf("error parsing response for SQL CLI version %w", err)
astroCliMinorVersion := semVerRegex.ReplaceAllString(astroCliVersion, trimmedMinorVersionRegexMatchString)
SQLCliCompatibility, exists = resp.SQLCliCompatibility[astroCliMinorVersion]
if !exists {
SQLCliCompatibility, exists = resp.SQLCliCompatibility[defaultVersionString]
if !exists {
return AstroSQLCliVersion{}, fmt.Errorf("error parsing response for SQL CLI version %w", err)
}
}
}
return AstroSQLCliVersion{SQLCliCompatibility.SQLCliVersion, SQLCliCompatibility.PreRelease}, nil
}
Expand Down
17 changes: 13 additions & 4 deletions sql/flow_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,19 @@ func TestGetPypiVersionInvalidHTTPRequestFailure(t *testing.T) {
assert.ErrorContains(t, err, expectedErrContains)
}

func TestGetPypiVersionInvalidAstroCliVersionFailure(t *testing.T) {
_, err := GetPypiVersion("https://raw.githubusercontent.com/astronomer/astro-sdk/astro-cli/sql-cli/config/astro-cli.json", "foo")
expectedErrContains := "error parsing response for SQL CLI version"
assert.ErrorContains(t, err, expectedErrContains)
func TestGetPypiVersionInvalidAstroCliVersionExactMatchSuccess(t *testing.T) {
_, err := GetPypiVersion("https://raw.githubusercontent.com/astronomer/astro-sdk/astro-cli/sql-cli/config/astro-cli.json", "1.9.0")
assert.NoError(t, err)
}

func TestGetPypiVersionInvalidAstroCliVersionMinorVersionMatchSuccess(t *testing.T) {
_, err := GetPypiVersion("https://raw.githubusercontent.com/astronomer/astro-sdk/astro-cli/sql-cli/config/astro-cli.json", "1.10.1")
assert.NoError(t, err)
}

func TestGetPypiVersionInvalidAstroCliVersionDefaultMatchSuccess(t *testing.T) {
_, err := GetPypiVersion("https://raw.githubusercontent.com/astronomer/astro-sdk/astro-cli/sql-cli/config/astro-cli.json", "x.y.z")
assert.NoError(t, err)
}

func TestGetBaseDockerImageURIInvalidURLFailure(t *testing.T) {
Expand Down