obmarg / kazan

Kubernetes API client for Elixir
https://hex.pm/packages/kazan
MIT License
140 stars 35 forks source link

New `Kazan.Models` Module Orgnanization for `ApiMachinery` #26

Closed mayppong closed 6 years ago

mayppong commented 6 years ago

So I'm working with kazan's master branch which has the new module organization changes. Based on the comment in the commit, I assume that the ObjectMeta would still be under the same location as it is now.

However, when I alias Kazan.Models.ApiMachinery.Apis.Meta.V1.ObjectMeta, I get this error message:

== Compilation error in file lib/ephemeral/kube.ex ==
** (CompileError) lib/ephemeral/kube.ex:28: Kazan.Models.ApiMachinery.Apis.Meta.V1.ObjectMeta.__struct__/1 is undefined, cannot expand struct Kazan.Models.ApiMachinery.Apis.Meta.V1.ObjectMeta
    (stdlib) lists.erl:1354: :lists.mapfoldl/3
    (stdlib) lists.erl:1355: :lists.mapfoldl/3
    (elixir) expanding macro: Kernel.@/1
    lib/ephemeral/kube.ex:28: Ephemeral.Kube (module)

I'm not sure where it is now in master branch so it would be nice if there was a little more description for models that are staying in the same module structure or if they changed in some other way unlike the other API models.

Thank you,

obmarg commented 6 years ago

Hey @mayppong - thanks for raising this.

I need to investigate when I get a bit more time, but this might have actually been a mistake when re-organising the various modules. For now you can find the ApiMachinery stuff under Kazan.Models.Apimachinery (note the small m in Apimachinery). So ObjectMeta is under Kazan.Models.Apimachinery.Meta.V1.ObjectMeta.

I suspect I'll end up changing things back to the old path in an hour or two, I just want to make sure of a few things first.

obmarg commented 6 years ago

Actually, I think this was done as part of #13. The k8s specs don't expose names in a way that's easy to automatically convert to CamelCase. Prior to #13 we had a handful of places where we'd manually converted, ApiMachinery being one of these places. However, since we didn't do it everywhere it seemed like it'd be more consistent to keep all modules with just the first letter uppercased, the rest lower case.

However, I'm not entirely sure I want to keep things like this. Maybe ApiMachinery should be one of the exceptions. What do you think @mayppong ?

At the very least I need to document this change in the changelog - obviously haven't done a good job of that yet.

mayppong commented 6 years ago

I don't think it's a big deal to use Apimachinery. I prefer consistency and predictability over looking nice.