Skip to content

Commit

Permalink
Remove Unused Error Results (#3033)
Browse files Browse the repository at this point in the history
  • Loading branch information
martinmaly authored Apr 22, 2022
1 parent efab12b commit e2526c5
Show file tree
Hide file tree
Showing 9 changed files with 23 additions and 77 deletions.
17 changes: 3 additions & 14 deletions porch/apiserver/pkg/registry/porch/packagecommon.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,7 @@ func (r *packageCommon) getPackageRevision(ctx context.Context, name string, opt
return nil, err
}

obj, err := pkg.GetPackageRevision()
if err != nil {
return nil, err
}
obj := pkg.GetPackageRevision()
return obj, nil
}

Expand All @@ -147,12 +144,7 @@ func (r *packageCommon) updatePackageRevision(ctx context.Context, name string,
return nil, false, err
}

oldObj, err := oldPackage.GetPackageRevision()
if err != nil {
klog.Infof("update failed to retrieve old object: %v", err)
return nil, false, err
}

oldObj := oldPackage.GetPackageRevision()
newRuntimeObj, err := objInfo.UpdatedObject(ctx, oldObj)
if err != nil {
klog.Infof("update failed to construct UpdatedObject: %v", err)
Expand Down Expand Up @@ -199,9 +191,6 @@ func (r *packageCommon) updatePackageRevision(ctx context.Context, name string,
return nil, false, apierrors.NewInternalError(err)
}

created, err := rev.GetPackageRevision()
if err != nil {
return nil, false, apierrors.NewInternalError(err)
}
created := rev.GetPackageRevision()
return created, false, nil
}
16 changes: 3 additions & 13 deletions porch/apiserver/pkg/registry/porch/packagerevision.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,7 @@ func (r *packageRevisions) List(ctx context.Context, options *metainternalversio
}

if err := r.packageCommon.listPackages(ctx, filter, func(p repository.PackageRevision) error {
item, err := p.GetPackageRevision()
if err != nil {
return err
}
item := p.GetPackageRevision()
result.Items = append(result.Items, *item)
return nil
}); err != nil {
Expand Down Expand Up @@ -142,10 +139,7 @@ func (r *packageRevisions) Create(ctx context.Context, runtimeObject runtime.Obj
return nil, apierrors.NewInternalError(err)
}

created, err := rev.GetPackageRevision()
if err != nil {
return nil, apierrors.NewInternalError(err)
}
created := rev.GetPackageRevision()
return created, nil
}

Expand Down Expand Up @@ -186,11 +180,7 @@ func (r *packageRevisions) Delete(ctx context.Context, name string, deleteValida
return nil, false, err
}

oldObj, err := oldPackage.GetPackageRevision()
if err != nil {
klog.Infof("update failed to retrieve old object: %v", err)
return nil, false, err
}
oldObj := oldPackage.GetPackageRevision()

if deleteValidation != nil {
err := deleteValidation(ctx, oldObj)
Expand Down
5 changes: 1 addition & 4 deletions porch/engine/pkg/engine/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -316,10 +316,7 @@ func (cad *cadEngine) UpdatePackageResources(ctx context.Context, repositoryObj
ctx, span := tracer.Start(ctx, "cadEngine::UpdatePackageResources", trace.WithAttributes())
defer span.End()

rev, err := oldPackage.GetPackageRevision()
if err != nil {
return nil, fmt.Errorf("failed to get package revision: %w", err)
}
rev := oldPackage.GetPackageRevision()

// Validate package lifecycle. Can only update a draft.
switch lifecycle := rev.Spec.Lifecycle; lifecycle {
Expand Down
5 changes: 1 addition & 4 deletions porch/repository/pkg/cache/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,7 @@ func TestLatestPackages(t *testing.T) {

gotLatest := map[string]string{}
for _, pr := range revisions {
rev, err := pr.GetPackageRevision()
if err != nil {
t.Errorf("GetPackageRevision(%s) failed: %v", pr.Key(), err)
}
rev := pr.GetPackageRevision()

if latest, ok := rev.Labels[api.LatestPackageRevisionKey]; ok {
if got, want := latest, api.LatestPackageRevisionValue; got != want {
Expand Down
9 changes: 3 additions & 6 deletions porch/repository/pkg/cache/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,18 +53,15 @@ type cachedPackageRevision struct {
isLatestRevision bool
}

func (c *cachedPackageRevision) GetPackageRevision() (*v1alpha1.PackageRevision, error) {
rev, err := c.PackageRevision.GetPackageRevision()
if err != nil {
return nil, err
}
func (c *cachedPackageRevision) GetPackageRevision() *v1alpha1.PackageRevision {
rev := c.PackageRevision.GetPackageRevision()
if c.isLatestRevision {
if rev.Labels == nil {
rev.Labels = map[string]string{}
}
rev.Labels[v1alpha1.LatestPackageRevisionKey] = v1alpha1.LatestPackageRevisionValue
}
return rev, nil
return rev
}

var _ repository.PackageRevision = &cachedPackageRevision{}
Expand Down
38 changes: 7 additions & 31 deletions porch/repository/pkg/git/git_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -375,10 +375,7 @@ func TestListPackagesTrivial(t *testing.T) {
t.Fatalf("draft.Close() failed: %v", err)
}

result, err := newRevision.GetPackageRevision()
if err != nil {
t.Fatalf("GetPackageRevision() failed: %v", err)
}
result := newRevision.GetPackageRevision()
if got, want := result.Spec.Lifecycle, v1alpha1.PackageRevisionLifecycleDraft; got != want {
t.Errorf("Newly created package type: got %q, want %q", got, want)
}
Expand Down Expand Up @@ -462,10 +459,7 @@ func TestCreatePackageInTrivialRepository(t *testing.T) {
t.Fatalf("draft.Close() failed: %v", err)
}

result, err := newRevision.GetPackageRevision()
if err != nil {
t.Fatalf("GetPackageRevision() failed: %v", err)
}
result := newRevision.GetPackageRevision()
if got, want := result.Spec.Lifecycle, v1alpha1.PackageRevisionLifecycleDraft; got != want {
t.Errorf("Newly created package type: got %q, want %q", got, want)
}
Expand Down Expand Up @@ -514,10 +508,7 @@ func TestListPackagesSimple(t *testing.T) {

got := map[repository.PackageRevisionKey]v1alpha1.PackageRevisionLifecycle{}
for _, r := range revisions {
rev, err := r.GetPackageRevision()
if err != nil {
t.Errorf("GetPackageRevision failed for %q: %v", r.KubeObjectName(), err)
}
rev := r.GetPackageRevision()
got[repository.PackageRevisionKey{
Repository: rev.Spec.RepositoryName,
Package: rev.Spec.PackageName,
Expand Down Expand Up @@ -574,10 +565,7 @@ func TestListPackagesDrafts(t *testing.T) {

got := map[repository.PackageRevisionKey]v1alpha1.PackageRevisionLifecycle{}
for _, r := range revisions {
rev, err := r.GetPackageRevision()
if err != nil {
t.Errorf("GetPackageRevision failed for %q: %v", r.KubeObjectName(), err)
}
rev := r.GetPackageRevision()
got[repository.PackageRevisionKey{
Repository: rev.Spec.RepositoryName,
Package: rev.Spec.PackageName,
Expand Down Expand Up @@ -638,11 +626,7 @@ func TestApproveDraft(t *testing.T) {
t.Fatalf("Close failed: %v", err)
}

rev, err := new.GetPackageRevision()
if err != nil {
t.Fatalf("GetPackageRevision failed: %v", err)
}

rev := new.GetPackageRevision()
if got, want := rev.Spec.Lifecycle, v1alpha1.PackageRevisionLifecyclePublished; got != want {
t.Errorf("Approved package lifecycle: got %s, want %s", got, want)
}
Expand Down Expand Up @@ -690,10 +674,7 @@ func TestDeletePackages(t *testing.T) {
for len(all) > 0 {
// Delete one of the packages
deleting := all[0]
pr, err := deleting.GetPackageRevision()
if err != nil {
t.Fatalf("GetPackageRevision of %q failed: %v", deleting.KubeObjectName(), err)
}
pr := deleting.GetPackageRevision()
name := repository.PackageRevisionKey{Repository: pr.Spec.RepositoryName, Package: pr.Spec.PackageName, Revision: pr.Spec.Revision}

if rn, ok := wantDeletedRefs[name]; ok {
Expand Down Expand Up @@ -941,12 +922,7 @@ func TestNested(t *testing.T) {

got := map[string]v1alpha1.PackageRevisionLifecycle{}
for _, pr := range revisions {
rev, err := pr.GetPackageRevision()
if err != nil {
t.Errorf("GetPackageRevision(%s) failed: %v", pr.Key(), err)
continue
}

rev := pr.GetPackageRevision()
if rev.Spec.Revision == "main" {
// skip packages with "main" revision, to match the above simplified package discovery algo.
continue
Expand Down
4 changes: 2 additions & 2 deletions porch/repository/pkg/git/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func (p *gitPackageRevision) uid() types.UID {
return types.UID(fmt.Sprintf("uid:%s:%s", p.path, p.revision))
}

func (p *gitPackageRevision) GetPackageRevision() (*v1alpha1.PackageRevision, error) {
func (p *gitPackageRevision) GetPackageRevision() *v1alpha1.PackageRevision {
key := p.Key()

return &v1alpha1.PackageRevision{
Expand All @@ -94,7 +94,7 @@ func (p *gitPackageRevision) GetPackageRevision() (*v1alpha1.PackageRevision, er
Tasks: p.tasks,
},
Status: v1alpha1.PackageRevisionStatus{},
}, nil
}
}

func (p *gitPackageRevision) GetResources(ctx context.Context) (*v1alpha1.PackageRevisionResources, error) {
Expand Down
4 changes: 2 additions & 2 deletions porch/repository/pkg/oci/oci.go
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ func (p *ociPackageRevision) Key() repository.PackageRevisionKey {
}
}

func (p *ociPackageRevision) GetPackageRevision() (*v1alpha1.PackageRevision, error) {
func (p *ociPackageRevision) GetPackageRevision() *v1alpha1.PackageRevision {
key := p.Key()

return &v1alpha1.PackageRevision{
Expand All @@ -382,7 +382,7 @@ func (p *ociPackageRevision) GetPackageRevision() (*v1alpha1.PackageRevision, er

Tasks: p.tasks,
},
}, nil
}
}

func (p *ociPackageRevision) GetUpstreamLock() (kptfile.Upstream, kptfile.UpstreamLock, error) {
Expand Down
2 changes: 1 addition & 1 deletion porch/repository/pkg/repository/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ type PackageRevision interface {
Lifecycle() v1alpha1.PackageRevisionLifecycle

// GetPackageRevision returns the PackageRevision ("DRY") API representation of this package-revision
GetPackageRevision() (*v1alpha1.PackageRevision, error)
GetPackageRevision() *v1alpha1.PackageRevision

// GetResources returns the PackageRevisionResources ("WET") API representation of this package-revision
// TODO: return PackageResources or filesystem abstraction?
Expand Down

0 comments on commit e2526c5

Please sign in to comment.