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

Sql cli/compat matrix #1026

Merged
merged 4 commits into from
Jan 20, 2023
Merged
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: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@
.env
.idea/
.vscode/
airflow/Dockerfile
airflow/pod-config.yml
airflow_settings*
astro
astro-cli
cmd/dags/.airflowignore
cover.out
coverage.txt
dist/
Expand Down
4 changes: 4 additions & 0 deletions cmd/sql/e2e/flow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"strings"
"testing"

"github.com/astronomer/astro-cli/version"

sql "github.com/astronomer/astro-cli/cmd/sql"

"github.com/stretchr/testify/assert"
Expand All @@ -32,6 +34,7 @@ func chdir(t *testing.T, dir string) func() {

func execFlowCmd(args ...string) error {
cmd := sql.NewFlowCommand()
version.CurrVersion = "1.8"
cmd.SetArgs(args)
_, err := cmd.ExecuteC()
return err
Expand Down Expand Up @@ -157,6 +160,7 @@ func TestE2EFlowRunCmd(t *testing.T) {
}
for _, tc := range testCases {
t.Run(strings.Join(tc.args, " "), func(t *testing.T) {
version.CurrVersion = "1.8"
err := execFlowCmd("init", tc.args[3])
assert.NoError(t, err)

Expand Down
14 changes: 14 additions & 0 deletions cmd/sql/flow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
testUtil "github.com/astronomer/astro-cli/pkg/testing"
sql "github.com/astronomer/astro-cli/sql"
"github.com/astronomer/astro-cli/sql/mocks"
"github.com/astronomer/astro-cli/version"
"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/container"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -124,6 +125,15 @@ func chdir(t *testing.T, dir string) func() {
}

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

func execFlowCmdWrongVersion(args ...string) error {
version.CurrVersion = "foo"
cmd := NewFlowCommand()
cmd.SetArgs(args)
_, err := cmd.ExecuteC()
Expand All @@ -136,6 +146,10 @@ func TestFlowCmd(t *testing.T) {
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
20 changes: 12 additions & 8 deletions sql/flow.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/astronomer/astro-cli/pkg/input"
"github.com/astronomer/astro-cli/pkg/util"
"github.com/astronomer/astro-cli/sql/include"
"github.com/astronomer/astro-cli/version"
"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/container"
"github.com/docker/docker/pkg/archive"
Expand Down Expand Up @@ -106,19 +107,22 @@ var ExecuteCmdInDocker = func(cmd, mountDirs []string, returnOutput bool) (exitC
return statusCode, cout, fmt.Errorf("docker client initialization failed %w", err)
}

astroSQLCliVersion, err := getPypiVersion(astroSQLCLIProjectURL)
if err != nil {
return statusCode, cout, err
}

baseImage, err := getBaseDockerImageURI(astroSQLCLIConfigURL)
if err != nil {
fmt.Println(err)
}
astroSQLCliVersion, err := getPypiVersion(astroSQLCLIConfigURL, version.CurrVersion)
if err != nil {
return statusCode, cout, err
}
preReleaseOptString := ""
if astroSQLCliVersion.Prerelease {
preReleaseOptString = "--pre"
}

currentUser, _ := user.Current()

dockerfileContent := []byte(fmt.Sprintf(include.Dockerfile, baseImage, astroSQLCliVersion, currentUser.Username, currentUser.Uid, currentUser.Username))
dockerfileContent := []byte(fmt.Sprintf(include.Dockerfile, baseImage, astroSQLCliVersion.Version, preReleaseOptString, currentUser.Username, currentUser.Uid, currentUser.Username))
if err := Os().WriteFile(sqlCLIDockerfilePath, dockerfileContent, fileWriteMode); err != nil {
return statusCode, cout, fmt.Errorf("error writing dockerfile %w", err)
}
Expand Down Expand Up @@ -224,11 +228,11 @@ var EnsurePythonSdkVersionIsMet = func(promptRunner input.PromptRunner) error {
if err != nil {
return err
}
SQLCLIVersion, err := getPypiVersion(astroSQLCLIProjectURL)
SQLCLIVersion, err := getPypiVersion(astroSQLCLIConfigURL, version.CurrVersion)
if err != nil {
return err
}
requiredRuntimeVersion, requiredPythonSDKVersion, err := getPythonSDKComptability(astroSQLCLIConfigURL, SQLCLIVersion)
requiredRuntimeVersion, requiredPythonSDKVersion, err := getPythonSDKComptability(astroSQLCLIConfigURL, SQLCLIVersion.Version)
if err != nil {
return err
}
Expand Down
48 changes: 22 additions & 26 deletions sql/flow_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,8 @@ import (
"encoding/json"
"fmt"
"net/http"
"sort"

"github.com/hashicorp/go-version"
)

type pypiVersionResponse struct {
Releases map[string]json.RawMessage `json:"releases"`
}

type configResponse struct {
BaseDockerImage string `json:"baseDockerImage"`
}
Expand All @@ -23,8 +16,14 @@ type compatibilityVersions struct {
AstroSdkPython string `json:"astroSDKPythonVersion"`
}

type sqlCliCompatibilityVersions struct {
SQLCliVersion string `json:"sqlCliVersion"`
PreRelease bool `json:"preRelease"`
}

type compatibilityResponse struct {
Compatibility map[string]compatibilityVersions `json:"compatibility"`
Compatibility map[string]compatibilityVersions `json:"compatibility"`
SQLCliCompatibility map[string]sqlCliCompatibilityVersions `json:"astroCliCompatibility"`
}

const (
Expand All @@ -39,35 +38,32 @@ var (

var httpClient = &http.Client{}

func GetPypiVersion(projectURL string) (string, error) {
req, err := http.NewRequestWithContext(context.TODO(), http.MethodGet, projectURL, http.NoBody)
type AstroSQLCliVersion struct {
Version string
Prerelease bool
}

func GetPypiVersion(configURL, astroCliVersion string) (AstroSQLCliVersion, error) {
req, err := http.NewRequestWithContext(context.TODO(), http.MethodGet, configURL, http.NoBody)
if err != nil {
return "", fmt.Errorf("error creating HTTP request %w", err)
return AstroSQLCliVersion{}, fmt.Errorf("error creating HTTP request %w", err)
}
res, err := httpClient.Do(req)
if err != nil {
return "", fmt.Errorf("error getting latest release version for project url %s, %w", projectURL, err)
return AstroSQLCliVersion{}, fmt.Errorf("error getting latest release version for project url %s, %w", configURL, err)
}
defer res.Body.Close()

var resp pypiVersionResponse
var resp compatibilityResponse
err = json.NewDecoder(res.Body).Decode(&resp)
if err != nil {
return "", fmt.Errorf("error parsing response for project version %w", err)
return AstroSQLCliVersion{}, fmt.Errorf("error parsing response for project version %w", err)
}

versions := make([]*version.Version, len(resp.Releases))
index := 0
for release := range resp.Releases {
if v, err := version.NewVersion(release); err != nil {
fmt.Println(fmt.Errorf("error parsing release version %w", err))
} else {
versions[index] = v
index++
}
SQLCliCompatibility, exists := resp.SQLCliCompatibility[astroCliVersion]
if !exists {
return AstroSQLCliVersion{}, fmt.Errorf("error parsing response for SQL CLI version %w", err)
}
sort.Sort(sort.Reverse(version.Collection(versions)))
return versions[0].Original(), nil
return AstroSQLCliVersion{SQLCliCompatibility.SQLCliVersion, SQLCliCompatibility.PreRelease}, nil
}

func GetBaseDockerImageURI(configURL string) (string, error) {
Expand Down
12 changes: 9 additions & 3 deletions sql/flow_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,23 +9,29 @@ import (
)

func TestGetPypiVersionInvalidHostFailure(t *testing.T) {
_, err := GetPypiVersion("http://abcd")
_, err := GetPypiVersion("http://abcd", "0.1")
expectedErrContains := "error getting latest release version for project url http://abcd, Get \"http://abcd\": dial tcp: lookup abcd"
assert.ErrorContains(t, err, expectedErrContains)
}

func TestGetPypiVersionInvalidJSONFailure(t *testing.T) {
_, err := GetPypiVersion("https://pypi.org")
_, err := GetPypiVersion("https://pypi.org", "0.1")
expectedErrContains := "error parsing response for project version invalid character '<' looking for beginning of value"
assert.ErrorContains(t, err, expectedErrContains)
}

func TestGetPypiVersionInvalidHTTPRequestFailure(t *testing.T) {
_, err := GetPypiVersion("gs://pyconuk-workshop/")
_, err := GetPypiVersion("gs://pyconuk-workshop/", "0.1")
expectedErrContains := "error getting latest release version for project url gs://pyconuk-workshop/"
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 TestGetBaseDockerImageURIInvalidURLFailure(t *testing.T) {
_, err := GetBaseDockerImageURI("http://efgh")
expectedErrContains := "error retrieving the latest configuration http://efgh, Get \"http://efgh\": dial tcp: lookup efgh"
Expand Down
11 changes: 7 additions & 4 deletions sql/flow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import (
"strings"
"testing"

"github.com/astronomer/astro-cli/version"

"github.com/astronomer/astro-cli/sql/mocks"
"github.com/astronomer/astro-cli/sql/testutil"
"github.com/docker/docker/api/types"
Expand All @@ -34,8 +36,8 @@ var (
mockDisplayMessagesErr = func(r io.Reader) error {
return errMock
}
mockGetPypiVersionErr = func(projectURL string) (string, error) {
return "", errMock
mockGetPypiVersionErr = func(configUrl string, cliVersion string) (AstroSQLCliVersion, error) {
return AstroSQLCliVersion{}, errMock
}
mockBaseDockerImageURIErr = func(astroSQLCLIConfigURL string) (string, error) {
return "", errMock
Expand All @@ -44,8 +46,8 @@ var (
mockGetAstroDockerfileRuntimeVersion = func() (string, error) {
return "7.1.0", nil
}
mockGetPypiVersion = func(projectURL string) (string, error) {
return "", nil
mockGetPypiVersion = func(configUrl string, cliVersion string) (AstroSQLCliVersion, error) {
return AstroSQLCliVersion{}, nil
}
mockGetPythonSDKComptabilityUnMatch = func(configURL, sqlCliVersion string) (astroRuntimeVersion, astroSDKPythonVersion string, err error) {
return ">7.1.0", "==1.3.0", nil
Expand Down Expand Up @@ -84,6 +86,7 @@ func TestExecuteCmdInDockerWithReturnValue(t *testing.T) {
return mockDockerBinder, nil
}
DisplayMessages = mockDisplayMessagesNil
version.CurrVersion = "1.8"
_, output, err := ExecuteCmdInDocker(testCommand, nil, true)
assert.NoError(t, err)

Expand Down
2 changes: 1 addition & 1 deletion sql/include/dockerfile_content.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ ENV AIRFLOW__ASTRONOMER__UPDATE_CHECK_INTERVAL 0
RUN apt-install-and-clean \
build-essential

RUN pip install astro-sql-cli==%s
RUN pip install "astro-sql-cli %s" %s

RUN id -u %s 2>/dev/null || useradd --uid %s --create-home %s
# This is necessary to run the docker image in GNU Linux since Astro CLI 1.8
Expand Down