search

appc / spec

App Container Specification and Tooling (archived, see https://github.com/rkt/rkt/issues/4024)
Apache License 2.0
1.26k stars 146 forks source link

k8s.io/kubernetes/pkg/api/resource is going to move, cyclic dependency #683

Closed sttts closed 7 years ago

sttts commented 7 years ago

.... to k8s.io/apimachinery/pkg/api/resource. But as Kubernetes vendors appc/spec, we have a cyclic dependency problem:

A solution might be to make a copy of the Quantity type here, cutting off the dependency.

sttts commented 7 years ago

/cc @s-urbaniak

lucab commented 7 years ago

Yes, I agree on cutting off the dependency. This is giving us issues in other places down the chain due to overly broad un-pinned dependencies, like in https://github.com/appc/docker2aci/pull/238.

s-urbaniak commented 7 years ago

I am fine with cutting the dependency. I made a quick scan over rkt and it seems we don't reference this type directly in rkt, so there should be no change necessary there.

lucab commented 7 years ago

I think we consume it for isolators-related logic in docker2aci (still), and for rkt we used consume more things re-exported more here. https://github.com/appc/spec/pull/672 has some history and references.

@sttts @dgonyeo who wants to cut it?

sttts commented 7 years ago

@lucab just go ahead cutting it. For the moment I leave pkg/api/resource in the kube repo as a temporary copy such that you have time to cut the link. Would be great if you can manage to do that in a few-days timeframe.

jonboulle commented 7 years ago

@sttts do you have time to put up a PR?

Dr. Stefan Schimanski notifications@github.com schrieb am Mi., 25. Jan. 2017, 16:04:

@lucab https://github.com/lucab just go ahead cutting it. For the moment I leave pkg/api/resource in the kube repo as a temporary copy such that you have time to cut the link. Would be great if you can manage to do that in a few-days timeframe.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/appc/spec/issues/683#issuecomment-275131323, or mute the thread https://github.com/notifications/unsubscribe-auth/ACewN2vrMS95G0z9roMA3ualdgahTEhBks5rV2R5gaJpZM4LtdJm .

sttts commented 7 years ago

@jonboulle I can create a minimal PR switching to apimachinery once that merged in kube.

If you want to get rid of the dependency completely, some duplication of the kube code is necessary. As the type Quantity (that is used here) has some more deps to kubernetes, you have to see what you really need for your context.