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

re-define noEngine annotation #1976

Closed
chengcheng-pei opened this issue Jun 18, 2020 · 1 comment · Fixed by #1970
Closed

re-define noEngine annotation #1976

chengcheng-pei opened this issue Jun 18, 2020 · 1 comment · Fixed by #1970
Assignees
Milestone

Comments

@chengcheng-pei
Copy link
Contributor

chengcheng-pei commented Jun 18, 2020

this is original codes:
_, noEngine := p.Annotations[machinelearningv1.ANNOTATION_NO_ENGINE]

if machinelearningv1.ANNOTATION_NO_ENGINE in annotations, noEngine is true, otherwise, it is false.

This is confusing. we need check the value of p.Annotations[machinelearningv1.ANNOTATION_NO_ENGINE] instead of whether it exists in the annotations map or not.

there is same concern about _, hasSeparateEnginePod := mlDep.Spec.Annotations[machinelearningv1.ANNOTATION_SEPARATE_ENGINE].

Please leave comments, so that I can update the pr: #1970

@chengcheng-pei chengcheng-pei added the triage Needs to be triaged and prioritised accordingly label Jun 18, 2020
@ukclivecox
Copy link
Contributor

I see you have created an initial PR. I will update to WIP and link to this issue. I agree its presently usinging exitence rather than boolean which makes more sense I think.

@ukclivecox ukclivecox removed the triage Needs to be triaged and prioritised accordingly label Jul 9, 2020
@ukclivecox ukclivecox added this to the 1.3 milestone Jul 9, 2020
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

Successfully merging a pull request may close this issue.

2 participants