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

Fields with default not always set in the resource config during discover #1925

Open
travjenkins opened this issue Feb 6, 2025 · 2 comments
Labels
bug Something isn't working wontfix This will not be worked on

Comments

@travjenkins
Copy link
Member

travjenkins commented Feb 6, 2025

I could see how this might not be a bug... but kind of feels like one to me and also causes an issue in the UI.

The source-postgres connector has a resource config schema that contains this property:

    "mode": {
      "type": "string",
      "enum": [
        "",
        "Normal",
        "Precise",
        "Only Changes",
        "Without Primary Key"
      ],
      "title": "Backfill Mode",
      "description": "How the preexisting contents of the table should be backfilled. This should generally not be changed.",
      "default": ""
    }

However during discover this default is not set on any of the bindings.

"bindings": [
      {
          "resource": {
              "namespace": "public",
              "stream": "bigtables00"
          },
          "target": "acme/foo/public/bigtables00"
      },
      {
          "resource": {
              "namespace": "public",
              "stream": "bigtables01"
          },
          "target": "acme/foo/public/bigtables01"
      },
      ...

This can cause problems in the UI as our schema validation tool ajv is setup to insert defaults when a property is not there. We could look into turning this off only for the edit flow - but I first wanted to gauge people's thoughts.

I think that having the empty string being a value is okay but marking it as the default seems unnecessary since the user could just leave that prop off all together.

@travjenkins travjenkins added the bug Something isn't working label Feb 6, 2025
@travjenkins travjenkins changed the title Postgres not setting default for mode in the resource config during discover source-postgres not setting default for mode in the resource config during discover Feb 6, 2025
@travjenkins travjenkins transferred this issue from estuary/connectors Feb 6, 2025
@travjenkins travjenkins changed the title source-postgres not setting default for mode in the resource config during discover Fields with default not always set in the resource config during discover Feb 6, 2025
@travjenkins
Copy link
Member Author

We chatted in post stand up and since everyone agrees that this is not a huge issue and does not impact a lot of users we will not take any action on this.

The true intent for default in json schema is that it is stating the value that will be used if the prop is NOT in the config. This is not how the UI consumes this as it will use the default value and insert that value into the config if it is missing.

A true technically correct solution to this concern would be for the UI to have a good UI/UX to make it clear to a user that they can safely leave things blank and the connector will figure out the best value for them.

@travjenkins travjenkins added the wontfix This will not be worked on label Feb 7, 2025
@travjenkins
Copy link
Member Author

Marking as wontfix for not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

1 participant