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.

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

Better integration of Custom Resources #54

Closed
mortenlj opened this issue Oct 23, 2023 · 3 comments
Closed

Better integration of Custom Resources #54

mortenlj opened this issue Oct 23, 2023 · 3 comments

Comments

@mortenlj
Copy link

When working with Custom Resources, it is some times useful to have the full power of a defined model, just like when working when "regular" resources. From what I can tell, I can define my custom resources by creating a model and a resource in the same way that the regular resources are created:

@dataclass
class OpenSearch(DataclassDictMixIn):
    apiVersion: 'str' = None
    kind: 'str' = None
    metadata: 'meta_v1.ObjectMeta' = None
    spec: 'OpenSearchSpec' = None
    status: 'OpenSearchStatus' = None


class OpenSearch(res.NamespacedResourceG, models.OpenSearch):
    _api_info = res.ApiInfo(
        resource=res.ResourceDef('aiven.io', 'v1alpha1', 'OpenSearch'),
        plural='opensearches',
        verbs=['delete', 'deletecollection', 'get', 'global_list', 'global_watch', 'list', 'patch', 'post', 'put', 'watch'],
    )

However, while this allows working with them directly, it fails when trying to load them from YAML, because the load mechanism assumes it's either a generic resource (with no proper model), or a resource defined in a submodule of lightkube.resources.

Have you considered to make it possible to find a resource definition by looking at subclasses1 of NamespacedResourceG?
That way, I could load my resources from YAML, as long as I have made sure to import my resource/model definition before calling load_from_yaml, and get fully typed objects from the loader.

Footnotes

  1. https://docs.python.org/3/library/stdtypes.html#class.__subclasses__

@gtsystem
Copy link
Owner

Would it work for you if we introduce a less magic way to register resources? The idea to use __subclasses__ is interesting, but it may lead to surprises. For example, what to do if there are multiple classes representing the same resource type? What if they have different attributes? What if the user want to limit the custom resources that can be loaded?

Proposal:

from lightkube.registry import register_resource

register_resource(OpenSearch)  # this is registering the new resource and it will fail if there is already registered an equivalent class

load_from_yaml(..)

The same function could also be used as a decorator:

from lightkube.registry import register_resource

@register_resource
class OpenSearch(res.NamespacedResourceG, models.OpenSearch)
   ...

@mortenlj
Copy link
Author

I like that proposal. 👍

After thinking more about it, I realised that, at least for now, I can circumvent the problem because I always knows which resource will be in the YAML. That means I can use the from_dict classmethod of my resource to load it, but in the future I probably would like to use load_all_yaml, so I still think this is a good improvement.

The explicit register approach would work well I think, and as you say, there would be less surprises.

gtsystem added a commit that referenced this issue Oct 29, 2023
gtsystem added a commit that referenced this issue Oct 29, 2023
@gtsystem
Copy link
Owner

New functionality is available in v0.15.0.

However the example above need to be corrected a bit:

from lightkube.codecs import resource_registry

resource_registry.register(OpenSearch)  # this is registering the new resource and it will fail if there is already registered an equivalent class

load_from_yaml(..)

Registry documentation: https://lightkube.readthedocs.io/en/latest/codecs/#reference

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants