Skip to content
This repository has been archived by the owner on Oct 22, 2021. It is now read-only.

Remove Entanglement code from Quarks Job #78

Merged
merged 4 commits into from
Apr 30, 2020
Merged

Conversation

rohitsakala
Copy link
Contributor

@rohitsakala rohitsakala commented Apr 28, 2020

#172404604

I think we should not put entanglement logic inside QuarksJob since it is not the feature of QuarksJob

Part of cloudfoundry-incubator/quarks-operator#909

@rohitsakala rohitsakala force-pushed the rohit/restart branch 2 times, most recently from 42a65fd to 318ff26 Compare April 28, 2020 06:45
@rohitsakala rohitsakala changed the title Replace LabelEntanglement with AnnotationUpdateRef Remove Entanglement code from Quarks Job Apr 28, 2020
@rohitsakala rohitsakala force-pushed the rohit/restart branch 2 times, most recently from 9d8fa68 to 4e38d6c Compare April 29, 2020 04:59
@manno
Copy link
Member

manno commented Apr 29, 2020

Is it really possible to do this? I was under the impression, that the annotation has a generated value, which in case of 'FanOut' the cf-operator can not predict.

https://github.com/cloudfoundry-incubator/quarks-job/blob/master/pkg/kube/controllers/quarksjob/output_persistor.go#L150

This value depends on the keys in the secrets data. Does this really work for all use-cases, when it's the same value for all?

https://www.pivotaltracker.com/n/projects/2192232/stories/171433464 made that label necessary (I thought at the time;).

@manno manno added the question Further information is requested label Apr 29, 2020
@rohitsakala
Copy link
Contributor Author

Is it really possible to do this? I was under the impression, that the annotation has a generated value, which in case of 'FanOut' the cf-operator can not predict.

https://github.com/cloudfoundry-incubator/quarks-job/blob/master/pkg/kube/controllers/quarksjob/output_persistor.go#L150

This value depends on the keys in the secrets data. Does this really work for all use-cases, when it's the same value for all?

https://www.pivotaltracker.com/n/projects/2192232/stories/171433464 made that label necessary (I thought at the time;).

@manno I realiased there is nothing to update here after our conversation. Please re-review.

@rohitsakala rohitsakala merged commit 5842804 into master Apr 30, 2020
@rohitsakala rohitsakala deleted the rohit/restart branch April 30, 2020 12:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants