-
Notifications
You must be signed in to change notification settings - Fork 259
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
Fix LayerData not being usable for ComputeStorage package #1203
Fix LayerData not being usable for ComputeStorage package #1203
Conversation
Previously the LayerData structure in the computestorage package used definitions from the hcs schema from /internal so it was not actually possible to create a LayerData structure for an outside caller. This just creates local type aliases for hcsschema.Version and hcsschema.Layer so a client can create the structure now using computestorage.Version and computestorage.Layer respectively. Signed-off-by: Daniel Canter <dcanter@microsoft.com>
20484e5
to
d244780
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. I haven't tested it, but on inspection Version
and Layer
themselves consist only of primitive types, so I expect this is sufficient.
Do you think this is sufficient to close #1073? Or are there likely to be other schema elements that need to be exposed in other APIs and we should keep that ticket open for a general "Why is the schema internal" discussion?
I don't mind either way, this meets the use-case I opened #1073 to unblock.
As I mentioned there, access to hcsschema.SchemaV21()
or similar would I think be slightly more API-friendly than manually setting SchemaVersion.Major
and SchemaVersion.Minor
, but it's certainly not a problem this way.
@TBBle We should keep it open. There's a bigger discussion to be had around moving the schema out of internal which we'll want to have. |
Related work items: microsoft#1067, microsoft#1097, microsoft#1119, microsoft#1170, microsoft#1176, microsoft#1180, microsoft#1181, microsoft#1182, microsoft#1183, microsoft#1184, microsoft#1185, microsoft#1186, microsoft#1187, microsoft#1188, microsoft#1189, microsoft#1191, microsoft#1193, microsoft#1194, microsoft#1195, microsoft#1196, microsoft#1197, microsoft#1200, microsoft#1201, microsoft#1202, microsoft#1203, microsoft#1204, microsoft#1205, microsoft#1206, microsoft#1207, microsoft#1209, microsoft#1210, microsoft#1211, microsoft#1218, microsoft#1219, microsoft#1220, microsoft#1223
Previously the LayerData structure in the computestorage package used
definitions from the hcs schema from /internal so it was not actually possible
to create a LayerData structure for an outside caller.
This just creates local type aliases for hcsschema.Version and hcsschema.Layer
so a client can create the structure now using computestorage.Version and
computestorage.Layer respectively.
Signed-off-by: Daniel Canter dcanter@microsoft.com