astro-cli
astro-cli copied to clipboard
Use `semver.NewVersion` instead of `semver.MustParse`
Describe the bug
We are currently using the semver.MustParse
method to parse versions in CLI. This would work fine if the version is static and defined in the code, but if the version string is passed in some form from the user end, then semver.MustParse
will cause panic in case the version is invalid.
Ideally, we should avoid panic in our Go code, and so the idea is to use semver.NewVersion
instead of semver.MustParse
wherever we are getting the version string from the user's end, this would avoid code panics and give us errors which can be handled inside our code. Also if possible we should have a fallback in case of an error.
Ex:
Current Code in docker.go
Start
method
if version := semver.MustParse(airflowVersion); version != nil {
airflowDockerVersion = version.Major()
}
Expectation:
version, err := semver.NewVersion(airflowVersion)
if err != nil {
airflowDockerVersion = 2 // fallback assuming it's Airflow 2, in order to handle the error
} else {
airflowDockerVersion = version.Major()
}
What CLI Version did you experience this bug?
What Operating System is the above CLI installed on?
🪜 Steps To Reproduce
- In your local project's Dockerfile add an airflow version label with an invalid semver, ex:
LABEL io.astronomer.docker.airflow.version="2.4.0.dev20220719"
- try
astro dev start
- observer the panic in the code wrt semver library not being able to parse the image version
📸 Screenshots