From 0250f96a36c0abadd80bb4afd1baf26693a07fa8 Mon Sep 17 00:00:00 2001 From: Greg Neiheisel <1036482+schnie@users.noreply.github.com> Date: Fri, 20 Dec 2024 14:01:31 -0500 Subject: [PATCH] Reorganize PersistentRunE hooks --- airflow/docker_test.go | 38 +++++--------------------------------- cmd/airflow.go | 9 ++++++--- cmd/airflow_hooks.go | 5 ----- cmd/root.go | 2 +- cmd/root_hooks.go | 4 ++-- 5 files changed, 14 insertions(+), 44 deletions(-) diff --git a/airflow/docker_test.go b/airflow/docker_test.go index 7eb708864..17196c8e3 100644 --- a/airflow/docker_test.go +++ b/airflow/docker_test.go @@ -364,7 +364,7 @@ func (s *Suite) TestDockerComposeStart() { imageHandler.On("TagLocalImage", mock.Anything).Return(nil).Once() composeMock := new(mocks.DockerComposeAPI) - composeMock.On("Ps", mock.Anything, mockDockerCompose.projectName, api.PsOptions{All: true}).Return([]api.ContainerSummary{}, nil).Times(4) + composeMock.On("Ps", mock.Anything, mockDockerCompose.projectName, api.PsOptions{All: true}).Return([]api.ContainerSummary{}, nil).Times(2) composeMock.On("Up", mock.Anything, mock.Anything, mock.Anything).Return(nil).Twice() checkWebserverHealth = func(url string, timeout time.Duration) error { @@ -392,7 +392,7 @@ func (s *Suite) TestDockerComposeStart() { imageHandler.On("ListLabels").Return(map[string]string{airflowVersionLabelName: airflowVersionLabel}, nil).Times(2) composeMock := new(mocks.DockerComposeAPI) - composeMock.On("Ps", mock.Anything, mockDockerCompose.projectName, api.PsOptions{All: true}).Return([]api.ContainerSummary{}, nil).Times(2) + composeMock.On("Ps", mock.Anything, mockDockerCompose.projectName, api.PsOptions{All: true}).Return([]api.ContainerSummary{}, nil).Times(1) composeMock.On("Up", mock.Anything, mock.Anything, mock.Anything).Return(nil).Once() checkWebserverHealth = func(url string, timeout time.Duration) error { @@ -418,7 +418,7 @@ func (s *Suite) TestDockerComposeStart() { imageHandler.On("ListLabels").Return(map[string]string{airflowVersionLabelName: airflowVersionLabel}, nil).Times(2) composeMock := new(mocks.DockerComposeAPI) - composeMock.On("Ps", mock.Anything, mockDockerCompose.projectName, api.PsOptions{All: true}).Return([]api.ContainerSummary{}, nil).Times(2) + composeMock.On("Ps", mock.Anything, mockDockerCompose.projectName, api.PsOptions{All: true}).Return([]api.ContainerSummary{}, nil).Times(1) composeMock.On("Up", mock.Anything, mock.Anything, mock.Anything).Return(nil).Once() checkWebserverHealth = func(url string, timeout time.Duration) error { @@ -444,7 +444,7 @@ func (s *Suite) TestDockerComposeStart() { imageHandler.On("ListLabels").Return(map[string]string{airflowVersionLabelName: airflowVersionLabel}, nil).Times(2) composeMock := new(mocks.DockerComposeAPI) - composeMock.On("Ps", mock.Anything, mockDockerCompose.projectName, api.PsOptions{All: true}).Return([]api.ContainerSummary{}, nil).Times(2) + composeMock.On("Ps", mock.Anything, mockDockerCompose.projectName, api.PsOptions{All: true}).Return([]api.ContainerSummary{}, nil).Times(1) composeMock.On("Up", mock.Anything, mock.Anything, mock.Anything).Return(nil).Once() checkWebserverHealth = func(url string, timeout time.Duration) error { @@ -470,7 +470,7 @@ func (s *Suite) TestDockerComposeStart() { imageHandler.On("TagLocalImage", mock.Anything).Return(nil).Once() composeMock := new(mocks.DockerComposeAPI) - composeMock.On("Ps", mock.Anything, mockDockerCompose.projectName, api.PsOptions{All: true}).Return([]api.ContainerSummary{}, nil).Times(4) + composeMock.On("Ps", mock.Anything, mockDockerCompose.projectName, api.PsOptions{All: true}).Return([]api.ContainerSummary{}, nil).Times(2) composeMock.On("Up", mock.Anything, mock.Anything, mock.Anything).Return(nil).Twice() checkWebserverHealth = func(url string, timeout time.Duration) error { @@ -490,37 +490,12 @@ func (s *Suite) TestDockerComposeStart() { composeMock.AssertExpectations(s.T()) }) - s.Run("project already running", func() { - composeMock := new(mocks.DockerComposeAPI) - composeMock.On("Ps", mock.Anything, mockDockerCompose.projectName, api.PsOptions{All: true}).Return([]api.ContainerSummary{{ID: "test-webserver-id", State: "running"}}, nil).Once() - - mockDockerCompose.composeService = composeMock - - err := mockDockerCompose.Start("", "", "", "", false, false, waitTime, nil) - s.Contains(err.Error(), "cannot start, project already running") - - composeMock.AssertExpectations(s.T()) - }) - - s.Run("compose ps failure", func() { - composeMock := new(mocks.DockerComposeAPI) - composeMock.On("Ps", mock.Anything, mockDockerCompose.projectName, api.PsOptions{All: true}).Return([]api.ContainerSummary{}, errMockDocker).Once() - - mockDockerCompose.composeService = composeMock - - err := mockDockerCompose.Start("", "", "", "", false, false, waitTime, nil) - s.ErrorIs(err, errMockDocker) - - composeMock.AssertExpectations(s.T()) - }) - s.Run("image build failure", func() { noCache := false imageHandler := new(mocks.ImageHandler) imageHandler.On("Build", "", "", airflowTypes.ImageBuildConfig{Path: mockDockerCompose.airflowHome, NoCache: noCache}).Return(errMockDocker).Once() composeMock := new(mocks.DockerComposeAPI) - composeMock.On("Ps", mock.Anything, mockDockerCompose.projectName, api.PsOptions{All: true}).Return([]api.ContainerSummary{}, nil).Once() checkWebserverHealth = func(url string, timeout time.Duration) error { return nil @@ -543,7 +518,6 @@ func (s *Suite) TestDockerComposeStart() { imageHandler.On("ListLabels").Return(map[string]string{}, errMockDocker).Once() composeMock := new(mocks.DockerComposeAPI) - composeMock.On("Ps", mock.Anything, mockDockerCompose.projectName, api.PsOptions{All: true}).Return([]api.ContainerSummary{}, nil).Once() checkWebserverHealth = func(url string, timeout time.Duration) error { return nil @@ -566,7 +540,6 @@ func (s *Suite) TestDockerComposeStart() { imageHandler.On("ListLabels").Return(map[string]string{}, nil).Once() composeMock := new(mocks.DockerComposeAPI) - composeMock.On("Ps", mock.Anything, mockDockerCompose.projectName, api.PsOptions{All: true}).Return([]api.ContainerSummary{}, nil).Once() composeMock.On("Up", mock.Anything, mock.Anything, mock.Anything).Return(errMockDocker).Once() checkWebserverHealth = func(url string, timeout time.Duration) error { @@ -590,7 +563,6 @@ func (s *Suite) TestDockerComposeStart() { imageHandler.On("ListLabels").Return(map[string]string{airflowVersionLabelName: airflowVersionLabel}, nil).Twice() composeMock := new(mocks.DockerComposeAPI) - composeMock.On("Ps", mock.Anything, mockDockerCompose.projectName, api.PsOptions{All: true}).Return([]api.ContainerSummary{}, nil).Once() composeMock.On("Up", mock.Anything, mock.Anything, mock.Anything).Return(nil).Once() checkWebserverHealth = func(url string, timeout time.Duration) error { diff --git a/cmd/airflow.go b/cmd/airflow.go index ff3bdd0da..6f56dcaa9 100644 --- a/cmd/airflow.go +++ b/cmd/airflow.go @@ -9,13 +9,13 @@ import ( "strings" "time" - "github.com/astronomer/astro-cli/airflow/runtimes" - "github.com/astronomer/astro-cli/airflow" + "github.com/astronomer/astro-cli/airflow/runtimes" airflowversions "github.com/astronomer/astro-cli/airflow_versions" astrocore "github.com/astronomer/astro-cli/astro-client-core" astroplatformcore "github.com/astronomer/astro-cli/astro-client-platform-core" "github.com/astronomer/astro-cli/cloud/environment" + "github.com/astronomer/astro-cli/cmd/utils" "github.com/astronomer/astro-cli/config" "github.com/astronomer/astro-cli/context" "github.com/astronomer/astro-cli/houston" @@ -130,7 +130,10 @@ func newDevRootCmd(platformCoreClient astroplatformcore.CoreClient, astroCoreCli // so we set that configuration in this persistent pre-run hook. // A few sub-commands don't require this, so they explicitly // clobber it with a no-op function. - PersistentPreRunE: ConfigureContainerRuntime, + PersistentPreRunE: utils.ChainRunEs( + SetupLogging, + ConfigureContainerRuntime, + ), } cmd.AddCommand( newAirflowInitCmd(), diff --git a/cmd/airflow_hooks.go b/cmd/airflow_hooks.go index d2172c164..743d108f2 100644 --- a/cmd/airflow_hooks.go +++ b/cmd/airflow_hooks.go @@ -6,7 +6,6 @@ import ( "path/filepath" "github.com/astronomer/astro-cli/airflow/runtimes" - softwareCmd "github.com/astronomer/astro-cli/cmd/software" "github.com/astronomer/astro-cli/cmd/utils" "github.com/astronomer/astro-cli/config" "github.com/spf13/cobra" @@ -29,10 +28,6 @@ func ConfigureContainerRuntime(_ *cobra.Command, _ []string) error { if err != nil { return err } - - if err := softwareCmd.SetUpLogs(os.Stdout, verboseLevel); err != nil { - return err - } return nil } diff --git a/cmd/root.go b/cmd/root.go index 30f4ef49d..d5b3d78c2 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -66,7 +66,7 @@ func NewRootCmd() *cobra.Command { Welcome to the Astro CLI, the modern command line interface for data orchestration. You can use it for Astro, Astronomer Software, or Local Development.`, PersistentPreRunE: utils.ChainRunEs( - SetupLoggingPersistentPreRunE, + SetupLogging, CreateRootPersistentPreRunE(astroCoreClient, platformCoreClient), ), } diff --git a/cmd/root_hooks.go b/cmd/root_hooks.go index 640ebc540..b97606c01 100644 --- a/cmd/root_hooks.go +++ b/cmd/root_hooks.go @@ -18,9 +18,9 @@ import ( "github.com/spf13/cobra" ) -// SetupLoggingPersistentPreRunE is a pre-run hook shared between software & cloud +// SetupLogging is a pre-run hook shared between software & cloud // setting up log verbosity. -func SetupLoggingPersistentPreRunE(_ *cobra.Command, _ []string) error { +func SetupLogging(_ *cobra.Command, _ []string) error { return softwareCmd.SetUpLogs(os.Stdout, verboseLevel) }