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

Render a mapping of shapeID -> scala FQN into metadata #1436

Open
kubukoz opened this issue Mar 11, 2024 · 5 comments
Open

Render a mapping of shapeID -> scala FQN into metadata #1436

kubukoz opened this issue Mar 11, 2024 · 5 comments

Comments

@kubukoz
Copy link
Member

kubukoz commented Mar 11, 2024

Suggested in #1364 (comment) - render a metadata value that contains a mapping of the shape's original ID to its generated class/trait.

This will make it feasible to implement additional code generation on top of (well, next to) what smithy4s generates.

Implementation notes: we can probably reuse the existing metadata

$version: "2.0"

metadata smithy4sGenerated = [
  {
    smithy4sVersion: "0.18.9",
    namespaces: ["alloy", "smithy.waiters", "alloy.common", "smithy.api"]
  }
]

by adding an optional (for backward compat) entry for the shape mapping:

$version: "2.0"

metadata smithy4sGenerated = [
  {
    smithy4sVersion: "0.18.9",
    namespaces: ["alloy", "smithy.waiters", "alloy.common", "smithy.api"],
    shapes: {
      "smithy.api#documentation": "smithy.api.Documentation",
      "smithy.api#http": "smithy.api.Http"
    }
  }
]

@Baccata does this work for you?

@Baccata
Copy link
Contributor

Baccata commented Mar 12, 2024

Yup

@daddykotex
Copy link
Contributor

I wonder how large this metadata entry is gonna get in practice. The nice thing is that it's in a file of its own, in the jar, that almost nobody looks at.

@kubukoz
Copy link
Member Author

kubukoz commented Mar 13, 2024

I brainstormed this with @zainab-ali and @majk-p, we were wondering if this is really a necessity. It appears that it depends on what the usecases really are.

For example, if we only need a way to derive a scala type's name from a ShapeId, we could expose the relevant function from codegen and achieve this with very few changes, no additions to the generated Smithy files, quickly and efficiently.

However, if the goal is to have an actual set of shapeIds in there (e.g. as a possible future replacement of namespaces), or to generate a StaticSchemaIndex of sorts, then we need at least the "key" side of things in that smithy4sGenerated entry.

From my side, the former would suffice.

Another problem is: in order to see the shapes in metadata, one has to invoke the Smithy4s codegen first and then load the generated Smithy files (or even hand-pick the smithy4s-generated.smithy file, which isn't that bad either). This isn't true if we have a public function in the codegen module: whatever "additional codegen" happens, it can run independently from smithy4s's codegen, which I would say is a plus (although not a game changer).


To sum up, I suggest that we take a different approach and split this:

  1. Provide a mapping function, e.g. def toScalaRef(shapeId: ShapeId): List[String]
  2. Provide the set of shapeIds that were generated, in the metadata:
$version: "2.0"

metadata smithy4sGenerated = [
  {
    smithy4sVersion: "0.18.9",
    namespaces: ["alloy", "smithy.waiters", "alloy.common", "smithy.api"],
    shapes: [
      "smithy.api#documentation",
      "smithy.api#http"
    ]
  }
]

This will be more space / smithy-parsing-efficient (only the keys have to be part of the metadata), and can be done incrementally. In addition, the tools that don't need the set of shapeIds don't have to bother loading the model after code generation.

Thoughts?

@Baccata
Copy link
Contributor

Baccata commented Mar 13, 2024

No thoughts, except maybe why do you want to store the shapes at all, in the metadata ? I think the assumption that everything was generated in a model is reasonably safe.

Anyway, I don't have much thoughts right now. Show me some code (even incomplete) and we'll talk.

@kubukoz
Copy link
Member Author

kubukoz commented Mar 13, 2024

why do you want to store the shapes at all, in the metadata

good point, if you see the metadata you likely see the rest of the model too.

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

No branches or pull requests

3 participants