Adds project-name as an optional argument for project initialization by schnie · Pull Request #1743 · astronomer/astro-cli

schnie · 2024-11-12T18:53:04Z

Description

This PR just allows the project name to be specified as an optional, positional argument. When used this way, we set the argument as the project name, and then additionally create a directory with the same name, and initialize a fresh project in that new directory. This change is purely additive, and existing commands and flags work the same as before.

Current

mkdir test-project
cd test-project
astro dev init --name test-project

New

astro dev init test-project
cd test-project

Example Usage

This change specifically:

❯ astro dev init etl
Initialized empty Astro project in /Users/schnie/repos/etl
To begin developing, change to your project directory with `cd etl`

Combined with #1728:

❯ astro dev init etl --from-template=etl
Initialized empty Astro project in /Users/schnie/repos/etl
To begin developing, change to your project directory with `cd etl`

🎟 Issue(s)

Resolves to https://github.com/astronomer/astro/issues/25473

🧪 Functional Testing

❯ astro dev init test-project
Initialized empty Astro project in /Users/schnie/repos/test-project
To begin developing, change to your project directory with `cd /Users/schnie/repos/test-project`

📸 Screenshots

Add screenshots to illustrate the validity of these changes.

📋 Checklist

Rebased from the main (or release if patching) branch (before testing)
Ran make test before taking out of draft
Ran make lint before taking out of draft
Added/updated applicable tests
Tested against Astro-API (if necessary).
Tested against Houston-API and Astronomer (if necessary).
Communicated to/tagged owners of respective clients potentially impacted by these changes.
Updated any related documentation

schnie · 2024-11-12T19:09:08Z

cmd/airflow.go

-	exists := config.ProjectConfigExists()
+	exists, err := config.IsProjectDir(config.WorkingPath)
+	if err != nil {
+		return err
+	}


Alternative function we already had to check if the project config file exists in the directory or not. The prior version assumes that the process is already within the working directory, where the alternative lets us check if the file exists at the certain path.

schnie · 2024-12-06T16:58:46Z

cmd/airflow.go

-		fmt.Printf(configReinitProjectConfigMsg+"\n", config.WorkingPath)
+		fmt.Printf(configReinitProjectConfigMsg, config.WorkingPath)
 	} else {
-		fmt.Printf(configInitProjectConfigMsg+"\n", config.WorkingPath)
+		fmt.Printf(configInitProjectConfigMsg, config.WorkingPath)
+	}


Just standardized the messages and the format strings and fixed a double vs single newline inconsistency.

schnie · 2024-12-06T17:01:30Z

cmd/airflow_test.go

-	s.Run("invalid args", func() {
-		cmd := newAirflowInitCmd()
-		cmd.Flag("name").Value.Set("test-project-name")
-		args := []string{"invalid-arg"}
-
-		r, stdin := s.mockUserInput("y")
-
-		// Restore stdin right after the test.
-		defer func() { os.Stdin = stdin }()
-		os.Stdin = r
-		err := airflowInit(cmd, args)
-		s.ErrorIs(err, errProjectNameSpaces)
-	})
-


This test is obsolete now that we accept a single positional argument. Cobra also will show help text if more than one positional argument is specified.

schnie · 2024-12-06T17:04:05Z

cmd/airflow.go

-		fmt.Printf("Initializing Astro project\nPulling Airflow development files from Astronomer Certified Airflow Version %s\n", defaultImageTag)
-	} else {
-		fmt.Printf("Initializing Astro project\nPulling Airflow development files from Astro Runtime %s\n", defaultImageTag)


These messages are not necessary and duplicative. I've also updated the corresponding test for this output.

schnie · 2024-12-06T17:05:08Z

cmd/errors.go

 	errInvalidBothCustomImageandVersion            = errors.New("you provided both a Custom image and a version. You have to provide only one of these to upgrade")                             //nolint

 	errConfigProjectName = errors.New("project name is invalid")
-	errProjectNameSpaces = errors.New("this project name is invalid, a project name cannot contain spaces. Try using '-' instead")


Obsolete error message. Cobra will show help text if more than one argument is specified.

neel-astro

Looks fine apart from one minor edge case :)

neel-astro · 2024-12-09T06:19:23Z

cmd/airflow.go

+	if len(args) > 0 {
+		projectName = args[0]
+	}


This might mean that if someone is trying astro dev init --name test project, would result in a project name as project when they might be looking for test project, should we at least warn and confirm if not error, when someone is setting both the flag and arg?

Good catch. Updated the logic here to catch this case and throw an error.

Also added a corresponding test.

schnie · 2024-12-09T16:44:53Z

cmd/airflow.go

+	// If the project name is specified with the --name flag,
+	// it cannot be specified as a positional argument as well, so return an error.
+	if projectName != "" && len(args) > 0 {
+		return "", errConfigProjectNameSpecifiedTwice
+	}


@neel-astro added this case here to throw an error message if both the flag and positional argument are used.

I also separated all the new and existing "project name" logic into this new function to make the linter happy. :)

…1743)

Adds project-name as an optional argument for init

31687d3

schnie force-pushed the feature/project-name-argument branch from f93f446 to 31687d3 Compare November 12, 2024 19:07

schnie commented Nov 12, 2024

View reviewed changes

schnie changed the title ~~Adds project-name as an optional argument for init~~ [WIP] Adds project-name as an optional argument for init Nov 12, 2024

schnie added 2 commits December 6, 2024 09:33

Merge branch 'main' into feature/project-name-argument

e053b24

Clean up

c1ae095

schnie changed the title ~~[WIP] Adds project-name as an optional argument for init~~ Adds project-name as an optional argument for project initialization Dec 6, 2024

schnie force-pushed the feature/project-name-argument branch from 2beaf76 to c840b7f Compare December 6, 2024 16:57

schnie commented Dec 6, 2024

View reviewed changes

schnie force-pushed the feature/project-name-argument branch from c840b7f to f8b6b72 Compare December 6, 2024 17:04

schnie commented Dec 6, 2024

View reviewed changes

schnie force-pushed the feature/project-name-argument branch 7 times, most recently from 8d649d8 to 69b2ea8 Compare December 6, 2024 19:24

Refactor and fix tests

4e82a74

schnie force-pushed the feature/project-name-argument branch from 69b2ea8 to 4e82a74 Compare December 6, 2024 19:28

schnie marked this pull request as ready for review December 6, 2024 19:35

schnie requested review from jeremybeard, kushalmalani, lzdanski and neel-astro as code owners December 6, 2024 19:35

schnie requested review from pritt20 and sam-awezec December 6, 2024 19:36

lzdanski removed their request for review December 6, 2024 21:06

lzdanski requested a review from yanmastin-astro December 6, 2024 21:06

neel-astro reviewed Dec 9, 2024

View reviewed changes

schnie added 2 commits December 9, 2024 11:22

Throw error if project name specified both ways

3573908

Fix linting errors

b3e89cb

schnie commented Dec 9, 2024

View reviewed changes

neel-astro approved these changes Dec 9, 2024

View reviewed changes

pritt20 approved these changes Dec 9, 2024

View reviewed changes

schnie merged commit b3db2fd into main Dec 9, 2024

schnie deleted the feature/project-name-argument branch December 9, 2024 17:24

neel-astro pushed a commit that referenced this pull request Dec 13, 2024

Adds project-name as an optional argument for project initialization (#…

315d0bf

…1743)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

Adds project-name as an optional argument for project initialization#1743

Adds project-name as an optional argument for project initialization#1743
schnie merged 6 commits intomainfrom
feature/project-name-argument

schnie commented Nov 12, 2024 •

edited

Loading

Uh oh!

schnie Nov 12, 2024

Uh oh!

schnie Dec 6, 2024

Uh oh!

schnie Dec 6, 2024

Uh oh!

schnie Dec 6, 2024

Uh oh!

schnie Dec 6, 2024

Uh oh!

neel-astro left a comment

Uh oh!

neel-astro Dec 9, 2024

Uh oh!

schnie Dec 9, 2024

Uh oh!

schnie Dec 9, 2024

Uh oh!

schnie Dec 9, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

Conversation

schnie commented Nov 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Current

New

Example Usage

🎟 Issue(s)

🧪 Functional Testing

📸 Screenshots

📋 Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

neel-astro left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

schnie commented Nov 12, 2024 •

edited

Loading