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.

Sign up for GitHub

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

Jump to bottom

HCP: error handling & empty source image id #90

Merged
merged 2 commits into from Dec 16, 2021
Merged

HCP: error handling & empty source image id #90

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eb9515f
this error is mainly ignored, and I think it would be better to panic…
azr Nov 23, 2021
52e55e7
don't error if no source image id was not passed
azr Nov 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 0 additions & 21 deletions packer/registry/image/image.go
View file
Open in desktop
Expand Up @@ -50,10 +50,6 @@ func (i *Image) Validate() error {
return errors.New("error registry image does not contain a valid ProviderName")
}

if i.SourceImageID == "" {
return errors.New("error registry image does not contain a valid SourceImageID")
}

azr marked this conversation as resolved.
Show resolved Hide resolved
return nil
}

Expand All @@ -70,7 +66,6 @@ func (i *Image) String() string {
// calling f(k,v). The func f is responsible for type asserting the expected type for the key and value before
// trying to create an Image from it.
func FromMappedData(mappedData interface{}, f func(key, value interface{}) (*Image, error)) ([]*Image, error) {

mapValue := reflect.ValueOf(mappedData)
if mapValue.Kind() != reflect.Map {
return nil, errors.New("error the incoming mappedData does not appear to be a map; found type to be" + mapValue.Kind().String())
Expand Down Expand Up @@ -118,10 +113,6 @@ func FromArtifact(a packer.Artifact, opts ...ArtifactOverrideFunc) (*Image, erro
// used to override the ProviderName for an existing Image.
func WithProvider(name string) func(*Image) error {
return func(img *Image) error {
if img == nil {
return errors.New("no go on empty image")
}

img.ProviderName = name
return nil
}
Expand All @@ -131,10 +122,6 @@ func WithProvider(name string) func(*Image) error {
// used to override the ImageId for an existing Image.
func WithID(id string) func(*Image) error {
return func(img *Image) error {
if img == nil {
return errors.New("no go on empty image")
}
Comment on lines -134 to -136
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was added to avoid a panic in case FromMappedData failed and returned a nil image. In this case, I think that this should panic, since the error was not checked. Also, this error is kind of weird.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(ditto throughout)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I'm also super open to suggestions!)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error is weird but panicking too. Maybe improving the error message is better 🤔 what do you think? Making it a friendly panic 😆


img.ImageID = id
return nil
}
Expand All @@ -144,10 +131,6 @@ func WithID(id string) func(*Image) error {
// used to override the SourceImageId for an existing Image.
func WithSourceID(id string) func(*Image) error {
return func(img *Image) error {
if img == nil {
return errors.New("no go on empty image")
}

img.SourceImageID = id
return nil
}
Expand All @@ -157,10 +140,6 @@ func WithSourceID(id string) func(*Image) error {
// used to override the ProviderRegion for an existing Image.
func WithRegion(region string) func(*Image) error {
return func(img *Image) error {
if img == nil {
return errors.New("no go on empty image")
}

img.ProviderRegion = region
return nil
}
Expand Down