Closed jacobtomlinson closed 4 months ago
Attention: Patch coverage is 91.66667%
with 1 line
in your changes missing coverage. Please review.
Project coverage is 95.30%. Comparing base (
87063fc
) to head (717ecb0
). Report is 121 commits behind head on main.
Files with missing lines | Patch % | Lines |
---|---|---|
kr8s/_objects.py | 80.00% | 1 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
The goal of this PR is to make it easier for folks to create custom API object subclasses.
Currently we support using
new_class
to generate a new object. But now that we support this automatically in #432 the only reasons to manually create a class are to either use classmethods like.get()
and.list()
, or to create custom methods on the subclass.Creating manual subclasses of
APIObject
in the sync API is a bit tricky as you need to jump through some hoops to get the sync wrapping working, hence why we recommend folks usekr8s.objects.new_class
which does this for you. But that kind of defeats the goal of creating objects with custom methods.If you subclass the class you generate with
new_class
the subclass is ignored becausekr8s._objects.get_class
returns the first match. This PR modifies the behaviour ofget_class
to return the last match, which allows you to subclass anyAPIObject
based class to override which one should be used. This means that users can usenew_class
to generate a base, and then subclass that to create their custom class. You don't even need to assign the base to anything, you can just use it in the class definitiion.This works nicely because the
CustomObject
class can call any of it's parent methods and they will be sync wrapped correctly and as far as the user if concerned they are just creating a new sync object.You can also do the same with the async version of
new_class
to create async objects.The only complexity would be if a user wants to create both a sync and async implementation like we provide with the core objects. In that case they would need to create an async class and then use the
kr8s._async_utils.sync()
decorator to convert it. They would also need to ensure the async methods are using theasync_
named methods on the base class so that they don't get clobbered when it is sync wrapped. In reality I don't think many folks will want to create both a sync and async class, and if they do it's probably better for them to define them both separately and suffer a small amount of duplication.In theory with this change users could also subclass core objects. For example if they wanted to extend
Pod
they could do so.This gives users access to more footguns, but also might give a bit more flexibility if they find a limitation in the
kr8s
implementations of objects. For example they could override an existing method and change it's behaviour, but that may break other code which is expecting a particular protocol. If people start doing this we might want to think about using more flexible protocols in places.