Export main types for v1 #147

runcom · 2016-06-16T09:09:27Z

Fix #133

/cc @stevvooe @vbatts @philips

This is basically how I expect those types to be exported and used in other tools.

philips · 2016-06-16T12:38:59Z

Two questions:

Can we add tests against the json schema to ensure these things can serialize and unserialize that correctly?
Are there any code gen programs that can generate structs from json schema?

runcom · 2016-06-16T12:42:42Z

my plan was to firstly agree on these two commits, then do what you're saying (and even adapt current code already present in the code base to use these types)
I'm not sure, will have to look around

runcom · 2016-06-16T12:44:06Z

bis, I basically did what's in runtime-spec atm and docker is using as well

philips · 2016-06-16T13:17:01Z

@runcom understood. A trivial review of the structs LGTM

vbatts · 2016-06-16T13:19:37Z

I looked all over for such a creature and wish there were a code-gen from json-schema. It would like be a tailored solution.

jonboulle · 2016-06-16T13:22:03Z

This seems totally crazy. What's the utility of json-schema if you can't
generate code?!? pah

On 16 June 2016 at 15:19, Vincent Batts [email protected] wrote:

I looked all over for such a creature and wish there were a code-gen
from json-schema. It would like be a tailored solution.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#147 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ACewNzkpcUUsjOTlkxqxZQXElfwiqFSJks5qMU1qgaJpZM4I3LSJ
.

philips · 2016-06-16T13:22:26Z

cc @casualjim know of any json schema to Go struct generators? Our schema files are here: https://github.com/opencontainers/image-spec/tree/master/schema

casualjim · 2016-06-16T13:39:29Z

You can generate code from it with go-swagger. There is only one construct in use that go-swagger currently doesn't support.

The oneOf here is currently not supported but we support a slightly different syntax to get that same effect (x-isnullable).
https://github.com/opencontainers/image-spec/blob/master/schema/defs-image.json#L76-L86

This should get the generation process going:

swagger generate model --help

runcom · 2016-06-16T13:44:31Z

Cool, we can work with that and maybe add a makefile target as well

vbatts · 2016-06-16T14:21:12Z

@casualjim thanks for the info

runcom · 2016-06-16T14:58:09Z

last commit does a simple generation of the defs in the ./schema dir (just a test so everyone can see how they look like)

stevvooe · 2016-06-16T17:22:54Z

specs/array_of_g_ids.go

+
+/*ArrayOfGIds array of g ids
+
+swagger:model ArrayOfGIDs


Aren't we already using jsonschema? Do we need to introduce yet another thing in here?

@stevvooe please refer to comments above about using swagger.. (this is just a test commit to discuss if this is the way we would go with this generation)

runcom · 2016-06-20T16:42:30Z

so this is what swagger generates w/o the validation - runcom@5e32077 - I still have to find a way to generate validation only go files (because --skip-struct does not seem to generate validation only, it just does nothing). Just wanted to gather acks on that commit and the try and add validation

vbatts · 2016-06-27T13:12:28Z

@runcom what's the latest here?

runcom · 2016-06-27T13:28:00Z

@vbatts basically types are generated in runcom@5e32077. Validation isn't coming with that PR because --skip-struct in swagger does not work correctly so I'm unsure on what to do next (given we have just the generated types..)

casualjim · 2016-06-27T23:57:46Z

Can you help me understand what is required? You want to split up the generated files in a file with the type definition and a file with the validation? I'd love to understand the reasons for it but let's start first with confirming intent.

Would it be possible to submit an issue when you run into a blocker with swagger?

wking · 2016-06-28T00:04:02Z

On Mon, Jun 27, 2016 at 04:57:46PM -0700, Ivan Porto Carrero wrote:

You want to split up the generated files in a file with the type
definition and a file with the validation?

Extrapolating from 1, I think the idea is to decouple the types
(which should be very stable) from validation code (which probably
doesn't need to be as stable).

It seems like that's what --skip-validator would be about, but we had
trouble figuring out what that did (or if it was wired up to anything
at all) 2.

casualjim · 2016-06-28T06:35:43Z

I've added support for --skip-struct to the generate model command

runcom · 2016-06-28T07:20:49Z

thx @casualjim - I've opened an Issue in swagger for validations

Signed-off-by: Antonio Murdaca <[email protected]>

runcom · 2016-06-28T16:16:22Z

Updated @vbatts @philips @stevvooe PTAL

philips · 2016-06-28T16:19:53Z

LGTM overall, two questions:

Should we be running go fmt over the generated code?
Is x-nullable sufficiently "standard" enough that other language generators will work?

runcom · 2016-06-28T16:22:04Z

need a bunch of test fixing, on it

runcom · 2016-06-28T16:23:56Z

actually looking at go-swagger/go-swagger#578

runcom · 2016-06-29T08:41:42Z

blocking again at ~~sgotti/glide-vc#16 (fixed a bug)~~ and at ~~go-swagger go-swagger/go-swagger#580~~ and go-swagger/go-swagger#581

casualjim · 2016-06-29T13:47:34Z

@philips x-nullable comes from the swagger folks as the recommended work-around, I've seen both x-isnullable and x-nullable in use so we support both. I hope that for oas 3.0 there will be full support for json schema instead of the cherry-picking that was done for 2.0

stevvooe · 2016-07-07T22:17:51Z

I'm still fairly uncomfortable with this PR. The generated code and validation are of low quality and we don't end up with very solid godoc output. Without language-specific directives polluting the schema, I am not sure there is a resolution to this.

It feels like this could have been done by now if we just did it by hand and validated against the schema after generation. Even with the generated code, we're still going to have to manually compare the specification with the generated implementations. If we can't get go-swagger to work in those cases, we'll end up

As far as I see it, there are a few options:

Hand generated structs and validators.
- Must auto-generate test cases and validate them against schema.
- Pain-staking.
- Easy to introduce inconsistencies
- By far, will be the best result
Fix go-swagger.
- May require backwards-incompatible changes to go-swagger.
- Language-specific formatting directives (see gogo/protobuf for a taste of that nightmare)
Write our own generator.
- May be a lot of work.
- Easy to special case generation
- Probably faster to write them by hand.

I'm sorry if this is not a majority opinion. In general, I agree with the sentiment of having this in place, so thank you @runcom for your work in this area.

Let me know if I can help in any way to make this go along faster.

stevvooe · 2016-07-07T22:18:43Z

specs-go/annotations.go

+type Annotations struct {
+
+
+  MapStringString


I know this is generated code, but this should be type Annotations map[string]string.

runcom · 2016-07-07T22:25:44Z

I tend to agree with @stevvooe here as I'm not totally fond about going with go-swagger here for the same reasons he told us above.

If anyone is interested I have the hand-written types in these patches:

Which is basically doing the same as the "runtime-spec" project - no generated codes for types right now

stevvooe · 2016-07-07T22:39:22Z

@runcom Those patches look great!

vbatts · 2016-07-18T19:51:43Z

What are the next steps here? Wait on using code generation, and pull in the hand done structs/types/etc?

runcom · 2016-07-18T20:23:57Z

What are the next steps here? Wait on using code generation, and pull in the hand done structs/types/etc?

I'd go for my hand written one as runtime-spec is already doing due to the lack of an appropriate and clean code generation tool

vbatts · 2016-07-18T22:09:03Z

Sounds good.

On Mon, Jul 18, 2016, 16:24 Antonio Murdaca [email protected]
wrote:

What are the next steps here? Wait on using code generation, and pull in
the hand done structs/types/etc?

I'd go for my hand written one as runtime-spec is already doing due to the
lack of an appropriate and clean code generation tool

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#147 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAEF6YWkHgoCr1N-4yKVFBA7UV_T4_8Eks5qW-DegaJpZM4I3LSJ
.

runcom force-pushed the specs-go branch 7 times, most recently from 83d3d6c to ee8ba91 Compare June 16, 2016 11:00

stevvooe reviewed Jun 16, 2016
View reviewed changes

runcom force-pushed the specs-go branch from 44b3cd8 to a42fb94 Compare June 16, 2016 17:46

wking mentioned this pull request Jun 20, 2016

image: Refactor to use cas/ref engines instead of walkers #159

Closed

add types and validations

8b58651

Signed-off-by: Antonio Murdaca <[email protected]>

runcom force-pushed the specs-go branch from a42fb94 to 8b58651 Compare June 28, 2016 16:16

stevvooe reviewed Jul 7, 2016
View reviewed changes

runcom mentioned this pull request Jul 18, 2016

Export types #172

Merged

vbatts closed this in #172 Jul 21, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Export main types for v1 #147

Export main types for v1 #147

runcom commented Jun 16, 2016 •

edited

Loading

philips commented Jun 16, 2016

runcom commented Jun 16, 2016

runcom commented Jun 16, 2016

philips commented Jun 16, 2016

vbatts commented Jun 16, 2016

jonboulle commented Jun 16, 2016

philips commented Jun 16, 2016

casualjim commented Jun 16, 2016

runcom commented Jun 16, 2016

vbatts commented Jun 16, 2016

runcom commented Jun 16, 2016

stevvooe Jun 16, 2016

runcom Jun 16, 2016

runcom commented Jun 20, 2016

vbatts commented Jun 27, 2016

runcom commented Jun 27, 2016

casualjim commented Jun 27, 2016

wking commented Jun 28, 2016

casualjim commented Jun 28, 2016

runcom commented Jun 28, 2016

runcom commented Jun 28, 2016

philips commented Jun 28, 2016 •

edited by caniszczyk

Loading

runcom commented Jun 28, 2016 •

edited

Loading

runcom commented Jun 28, 2016

runcom commented Jun 29, 2016 •

edited

Loading

casualjim commented Jun 29, 2016

stevvooe commented Jul 7, 2016

stevvooe Jul 7, 2016

runcom commented Jul 7, 2016 •

edited

Loading

stevvooe commented Jul 7, 2016

vbatts commented Jul 18, 2016

runcom commented Jul 18, 2016

vbatts commented Jul 18, 2016

Export main types for v1 #147

Export main types for v1 #147

Conversation

runcom commented Jun 16, 2016 • edited Loading

philips commented Jun 16, 2016

runcom commented Jun 16, 2016

runcom commented Jun 16, 2016

philips commented Jun 16, 2016

vbatts commented Jun 16, 2016

jonboulle commented Jun 16, 2016

philips commented Jun 16, 2016

casualjim commented Jun 16, 2016

runcom commented Jun 16, 2016

vbatts commented Jun 16, 2016

runcom commented Jun 16, 2016

stevvooe Jun 16, 2016

Choose a reason for hiding this comment

runcom Jun 16, 2016

Choose a reason for hiding this comment

runcom commented Jun 20, 2016

vbatts commented Jun 27, 2016

runcom commented Jun 27, 2016

casualjim commented Jun 27, 2016

wking commented Jun 28, 2016

casualjim commented Jun 28, 2016

runcom commented Jun 28, 2016

runcom commented Jun 28, 2016

philips commented Jun 28, 2016 • edited by caniszczyk Loading

runcom commented Jun 28, 2016 • edited Loading

runcom commented Jun 28, 2016

runcom commented Jun 29, 2016 • edited Loading

casualjim commented Jun 29, 2016

stevvooe commented Jul 7, 2016

stevvooe Jul 7, 2016

Choose a reason for hiding this comment

runcom commented Jul 7, 2016 • edited Loading

stevvooe commented Jul 7, 2016

vbatts commented Jul 18, 2016

runcom commented Jul 18, 2016

vbatts commented Jul 18, 2016

runcom commented Jun 16, 2016 •

edited

Loading

philips commented Jun 28, 2016 •

edited by caniszczyk

Loading

runcom commented Jun 28, 2016 •

edited

Loading

runcom commented Jun 29, 2016 •

edited

Loading

runcom commented Jul 7, 2016 •

edited

Loading