-
Notifications
You must be signed in to change notification settings - Fork 221
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
Prelude shapes are not available in JSON AST models built via smithy-build
#947
Comments
The prelude is, per the spec, inherently part of every model. So any parsers need to build in an understanding of them. Having the model exposed better would be a good thing to do still, though I'm not sure how we go about it. I'll chat with the team about this. In the meantime, is there any particular reason you're not using our typescript generators? |
Thanks for the response! This makes sense. JSON AST models shouldn't have the prelude shapes in them because the prelude is implicit, which kind of rules out option 1 I presented. It seems that the smithy library itself builds in it's knowledge of the prelude via loading/parsing the On your final point: Our code-generators are written in TypeScript and target Haskell as the language to generate for, which is why we're not using the typescript libraries you linked to, apologies for not being clear. Thanks |
I think something like a cli command could work. If we wanted to make it look like a normal smithy model package, we could create a separate project within the smithy repo that extracts the model from
Oh now that's interesting! What made you decide to go that route over using the Java tooling? |
It was a difficult choice to be honest. The primary motivating factors were:
|
This adds a configuration option to the node model serializer that disables filtering the prelude from the output. This is mostly to enable alternate implementation to base their prelude understanding off of a JSON AST version of the prelud model (see smithy-lang#947).
This adds a configuration option to the node model serializer that disables filtering the prelude from the output. This is mostly to enable alternate implementation to base their prelude understanding off of a JSON AST version of the prelud model (see #947).
Context
We’re starting to integrate
smithy
into our toolchain as a means to define interfaces and generate both clients and services. We’re authoring interfaces using the smithy IDL, building the models to the JSON AST using smithy-build/smithy-cli and passing the resultingmodel.json
as input to a code-generator we’re developing in-house. This code-generator is written in TypeScript and does not depend on any java base classes provided in thesmithy
repo.The issue
The built
model.json
that smithy-build emits (when running a “source” build: https://awslabs.github.io/smithy/1.0/guides/building-models/gradle-plugin.html#building-a-source-model) does not contain any of the shapes defined in the prelude (smithy.api
namespace). Prelude shapes are explicitly filtered out without an ability to override this behaviour.See:https://github.com/awslabs/smithy/blob/3a6a9a6ef43be5bc8768a7d8daef85f1a6df9a60/smithy-model/src/main/java/software/amazon/smithy/model/shapes/ModelSerializer.java#L57
Without the prelude shapes being present in the
model.json
it’s not possible to determine certain properties of the model.This isn’t an issue for any tooling that is built on-top of the java base classes provided because model assembly (via e.g.
ModelAssembler
andPrelude
classes) ensures that the prelude shapes are merged into the in-memory model which is then supplied to plugins; plugins get to see a fully realised model. This is an issue for consumers that only depend on themodel.json
when using source builds.Example
Given the interface:
A source build emits
model.json
containingThis JSON AST model contains a reference to
smithy.api#httpBasicAuth
but not its definintion. Without the definition of this prelude shape in the model then it can’t be determined whether or nothttpBasicAuth
is an authentication trait or not (by the presence of theauthDefinintion
trait on it). This is one part of determining what the effective authentication traits exist on a service. See: https://github.com/awslabs/smithy/blob/3a6a9a6ef43be5bc8768a7d8daef85f1a6df9a60/smithy-model/src/main/java/software/amazon/smithy/model/knowledge/ServiceIndex.java#L206This functionality cannot currently be implemented in our code-generator without the definition of the prelude shapes in the
model.json
.I don’t think any type of projection we could supply to
smithy-build
would emit the prelude in JSON AST format, prelude shapes are always filtered out.Proposals
We’re looking for some advice before we prep a PR on the best way to modify the code such that the prelude is available in JSON AST form so that we can make it available to our code-generator. There’s a few ways I can think to update the codebase to do this:
Add a command-line flag to smithy-cli’s
build
command, e.g.--emitPreludeShapes
to write the prelude shapes into themodel.json
. All existing consumers will not be supplying this flag so existing behvaviour will be preserved by default. This flag will effectively be threading some configuration down toModelSerializer
to allow the overriding of this line: https://github.com/awslabs/smithy/blob/3a6a9a6ef43be5bc8768a7d8daef85f1a6df9a60/smithy-model/src/main/java/software/amazon/smithy/model/shapes/ModelSerializer.java#L57The call stack from
smithy-cli
down to this layer of the code looks like this:The configuration parameter would have to be threaded through
SmithyBuild
,SmithyBuildImpl
,ModelPlugin
down toModelSerializer
. Thoughts on the cleanest way to do this?Add a whole new command to
smithy-cli
, something likeprelude-ast
, that simply emits the prelude as a JSON AST to standard-out (or a file maybe). If the prelude model JSON AST is available, even as a separate JSON structure/file, we can merge the prelude JSON into the existingmodel.json
at runtime within our code-generator.Any other suggestions welcome!
The text was updated successfully, but these errors were encountered: