Skip to content
This repository has been archived by the owner on Dec 7, 2023. It is now read-only.

Client and Storage rework: Recognize multiple API groups #221

Merged
merged 5 commits into from
Jul 19, 2019

Conversation

luxas
Copy link
Contributor

@luxas luxas commented Jul 18, 2019

Before this, the client and storage framework was locked in to only one particular combination of apiVersion, or in other words, GroupVersion. We only checked meta.Kind everywhere; but that always implicitly assumed that the GroupVersion was ignite/v1alpha1

Fixes: #199

This PR:

  • Makes the Storage (Cache), and Filterer interfaces operate on schema.GroupVersionKind, not meta.Kind
  • Automatically sets the desired GVK when decoding or creating an object, so the hacky defaulting functions we had are now obsolete
  • Extracts ignite-related stuff from the Client struct to a gv-specific IgniteInternalClient, and passes through the GVK automatically to the storage as a consequence.
  • Adds a Client.New -> Storage.New method being the canonical way to create a new API object. In other words, do not do &api.VM{} or similar anymore.
  • Adapts the rest of the application to this new code flow

@luxas luxas added kind/refactor Categorizes issue or PR as related to refactoring code. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. kind/enhancement Categorizes issue or PR as related to improving an existing feature. labels Jul 18, 2019
@luxas luxas added this to the v0.5.0 milestone Jul 18, 2019
Copy link
Contributor

@twelho twelho left a comment

Choose a reason for hiding this comment

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

A very welcome improvement 👍 just a couple of little cleanup suggestions.

log "github.com/sirupsen/logrus"
api "github.com/weaveworks/ignite/pkg/apis/ignite"
meta "github.com/weaveworks/ignite/pkg/apis/meta/v1alpha1"
"github.com/weaveworks/ignite/pkg/storage"
"github.com/weaveworks/ignite/pkg/storage/filterer"

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove blank line?

log "github.com/sirupsen/logrus"
api "github.com/weaveworks/ignite/pkg/apis/ignite"
meta "github.com/weaveworks/ignite/pkg/apis/meta/v1alpha1"
"github.com/weaveworks/ignite/pkg/storage"
"github.com/weaveworks/ignite/pkg/storage/filterer"

Copy link
Contributor

Choose a reason for hiding this comment

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

Rerun make tidy after removing the blank line from template

meta "github.com/weaveworks/ignite/pkg/apis/meta/v1alpha1"

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove two blank lines

@@ -5,6 +5,8 @@ import (

meta "github.com/weaveworks/ignite/pkg/apis/meta/v1alpha1"
"github.com/weaveworks/ignite/pkg/storage"

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove blank line

@luxas
Copy link
Contributor Author

luxas commented Jul 19, 2019

The blank lines are rebase conflict. I'll fixup those in this follow up: #224

@luxas luxas merged commit 6cef4e9 into weaveworks:master Jul 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/enhancement Categorizes issue or PR as related to improving an existing feature. kind/refactor Categorizes issue or PR as related to refactoring code. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a Client.Resource().New() method
2 participants