Skip to content

Add validation service#1099

Merged
NicolasMahe merged 3 commits intodevfrom
fix/service-validation
Jun 25, 2019
Merged

Add validation service#1099
NicolasMahe merged 3 commits intodevfrom
fix/service-validation

Conversation

@krhubert
Copy link
Copy Markdown
Contributor

@krhubert krhubert commented Jun 24, 2019

I've added back service validation but there is one pbm:

If we don't have "main" dependency as before how do we address it in volumeFrom? here

related to #1088

Comment thread service/service_validate.go Outdated
@NicolasMahe
Copy link
Copy Markdown
Member

If we don't have "main" dependency as before how do we address it in volumeFrom? here

I'm not sure to exactly understand, but: if there is no volume defined in the configuration ("main" dependency), then not volume will be mounted when use the volumeFrom.
volumeFrom only tell which dependency's volumes to mount from, not explicitly the volume themself. So if not volume are specified, then volumeFrom will mount 0 volume. No error.

Copy link
Copy Markdown
Member

@NicolasMahe NicolasMahe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comments

@krhubert krhubert force-pushed the fix/service-validation branch from 0ea064b to b7ee499 Compare June 25, 2019 05:38
@krhubert krhubert requested a review from NicolasMahe June 25, 2019 05:45
Copy link
Copy Markdown
Member

@NicolasMahe NicolasMahe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The validation seems good but the validation function is not called.
I guess it should be called from ServiceSDK.Create?

@NicolasMahe NicolasMahe merged commit a4ba68f into dev Jun 25, 2019
@NicolasMahe NicolasMahe deleted the fix/service-validation branch June 25, 2019 09:49
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 this pull request may close these issues.

2 participants