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

Internally Tagged Enum Definitions #39

Open
RXminuS opened this issue May 29, 2020 · 4 comments
Open

Internally Tagged Enum Definitions #39

RXminuS opened this issue May 29, 2020 · 4 comments

Comments

@RXminuS
Copy link

RXminuS commented May 29, 2020

Hey I'm trying to generate typescript types from the generated JSON schema and am running into a bit of a design constraint.

Basically the generated type for an enum that has [#serde(tag="...")] gets turned into an anyOf where the resulting types are directly mentioned without them being given a definition. This is in contrast from for instance using [#serde(tag="t", content="c")] where a new definition is inserted for every type of content.

When generating Typescript the latter is much more convenient since you get a discriminated union where you can still refer to each individual item since they will be named. In contrast the former just gives a giant union blob where it's hard to do things like casting to a specific type.

I would like to figure out a way we can make the JSON schema more verbose and essentially generate definitions for every enum item. Happy to make a PR if someone can point me in the right direction of where to find the relevant code bits.

@jetuk
Copy link

jetuk commented Nov 26, 2020

@RXminuS did you ever resolve this query?

@Tails
Copy link

Tails commented Jul 12, 2021

@GREsau what would the difficulty of implementing this be? If we have a checklist here we could work on it in a PR.

  • create test based on internally tagged enum to include discriminator
  • add 'discriminator' option to OpenAPI schema settings definition to enable/disable generation of property
  • find out where parsed serde attributes are stored / used -> expr_for_internal_tagged_enum()Consider implementing JsonSchema for RootSchema #191
  • make sure every instance has the 'type' property referred by the 'discriminator'
  • create Visitor implementation to add this property to schema
  • have Visitor set the discriminator based on whether the serde struct is tagged
  • ...

See https://github.com/parture-org/schemars/tree/openapi-discriminator for WIP

psFried added a commit to estuary/flow that referenced this issue Jan 30, 2023
Adds a new `CUSTOM` variant of storage mappings, which allows catalogs
to use a variety of S3-compatible storage services by specifying the
`endpoint` explicitly. This is not yet directly exposed to end-users,
since `storageMappings` are handled by the control plane. But it does
give us the ability to use custom storage endpoints by configuring the
storage mappings using something like:

```
{"stores":[{"provider":"CUSTOM","bucket":"the-bucket","endpoint":"some.storage.endpoint.com"}]}
```

Credentials are handled by using the tenant name of each task or
collection as a `profile` in the journal storage URI. This profile
is understood by the brokers and is looked up in `~/.aws/credentials` and
`~/.aws/config` to provide the credentials and region configuration.

In order to prevent any `CUSTOM` storage endpoints from using the
`default` aws config values, an additional validation was added
to ensure that tenant names cannot be `default`.

One thing to point is that the catalog JSON schema now isn't able to
mark the "provider" field as required, due to SchemaRS lacking
[support for internally tagged enums](GREsau/schemars#39).
I'm thinking this isn't actually a huge deal since end users don't edit
storage mappings in catalog specs anyway. So I'm inclined to leave that
as it is for right now.
psFried added a commit to estuary/flow that referenced this issue Jan 30, 2023
Adds a new `CUSTOM` variant of storage mappings, which allows catalogs
to use a variety of S3-compatible storage services by specifying the
`endpoint` explicitly. This is not yet directly exposed to end-users,
since `storageMappings` are handled by the control plane. But it does
give us the ability to use custom storage endpoints by configuring the
storage mappings using something like:

```
{"stores":[{"provider":"CUSTOM","bucket":"the-bucket","endpoint":"some.storage.endpoint.com"}]}
```

Credentials are handled by using the tenant name of each task or
collection as a `profile` in the journal storage URI. This profile
is understood by the brokers and is looked up in `~/.aws/credentials` and
`~/.aws/config` to provide the credentials and region configuration.

In order to prevent any `CUSTOM` storage endpoints from using the
`default` aws config values, an additional validation was added
to ensure that tenant names cannot be `default`.

One thing to point is that the catalog JSON schema now isn't able to
mark the "provider" field as required, due to SchemaRS lacking
[support for internally tagged enums](GREsau/schemars#39).
I'm thinking this isn't actually a huge deal since end users don't edit
storage mappings in catalog specs anyway. So I'm inclined to leave that
as it is for right now.
psFried added a commit to estuary/flow that referenced this issue Jan 30, 2023
Adds a new `CUSTOM` variant of storage mappings, which allows catalogs
to use a variety of S3-compatible storage services by specifying the
`endpoint` explicitly. This is not yet directly exposed to end-users,
since `storageMappings` are handled by the control plane. But it does
give us the ability to use custom storage endpoints by configuring the
storage mappings using something like:

```
{"stores":[{"provider":"CUSTOM","bucket":"the-bucket","endpoint":"some.storage.endpoint.com"}]}
```

Credentials are handled by using the tenant name of each task or
collection as a `profile` in the journal storage URI. This profile
is understood by the brokers and is looked up in `~/.aws/credentials` and
`~/.aws/config` to provide the credentials and region configuration.

In order to prevent any `CUSTOM` storage endpoints from using the
`default` aws config values, an additional validation was added
to ensure that tenant names cannot be `default`.

One thing to point is that the catalog JSON schema now isn't able to
mark the "provider" field as required, due to SchemaRS lacking
[support for internally tagged enums](GREsau/schemars#39).
I'm thinking this isn't actually a huge deal since end users don't edit
storage mappings in catalog specs anyway. So I'm inclined to leave that
as it is for right now.
@stevenliebregt
Copy link

This would be really nice to have

@tomas789
Copy link

I have done some research into this and summarized it here. It has a manual workaround to this issue. It would still be nice to have the PR finished up and merged 👍

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

5 participants