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

validate: add the validation of Resources.devices #374

Merged
merged 2 commits into from
Aug 30, 2017

Conversation

zhouhao3
Copy link

@zhouhao3 zhouhao3 commented Apr 28, 2017

According to the following spec:

devices.type
devices.access

Signed-off-by: zhouhao zhouhao@cn.fujitsu.com

Signed-off-by: zhouhao <zhouhao@cn.fujitsu.com>
Signed-off-by: zhouhao <zhouhao@cn.fujitsu.com>
@liangchenye
Copy link
Member

resources.devices.access?

r.Resources.Devices[index].Access))

@wking
Copy link
Contributor

wking commented Apr 28, 2017 via email

@Mashimiao
Copy link

Mashimiao commented May 5, 2017

LGTM
I think putting type or access value validation here is OK. It's hard to exactly check composition of r, w, m in JSON schema as I tried in runtime-spec/#690 and has been rejected.

Approved with PullApprove

@zhouhao3
Copy link
Author

zhouhao3 commented May 8, 2017

@liangchenye @mrunalp PTAL

@wking
Copy link
Contributor

wking commented May 8, 2017 via email

@zhouhao3
Copy link
Author

ping @liangchenye @mrunalp

@zhouhao3
Copy link
Author

@liangchenye @mrunalp PTAL

@liangchenye
Copy link
Member

We can drop this kind of validation when the JSON schema validation is adopted in runtime-tools.
Open #453 to track this.

@q384566678 @Mashimiao will you try to integrate that?

@liangchenye
Copy link
Member

liangchenye commented Aug 30, 2017

LGTM

Approved with PullApprove

1 similar comment
@Mashimiao
Copy link

Mashimiao commented Aug 30, 2017

LGTM

Approved with PullApprove

@Mashimiao Mashimiao merged commit 3767027 into opencontainers:master Aug 30, 2017
@zhouhao3 zhouhao3 deleted the linux-devices branch August 30, 2017 06:32
@zhouhao3
Copy link
Author

@liangchenye I will try to do this.

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.

4 participants