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

Remove stale required key in OpenAPI config #12341

Merged
merged 1 commit into from
Apr 28, 2022

Conversation

Ohcui
Copy link
Contributor

@Ohcui Ohcui commented Apr 26, 2022

What

There is a key in required list, but not in properties config, issue: #12340

How

Remove the key in required list.

Recommended reading order

  1. src/main/openapi/config.yaml: components.schemas.WebBackendConnectionCreate

@CLAassistant
Copy link

CLAassistant commented Apr 26, 2022

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added area/api Related to the api area/platform issues related to the platform labels Apr 26, 2022
@alafanechere alafanechere self-assigned this Apr 27, 2022
@alafanechere alafanechere mentioned this pull request Apr 28, 2022
@alafanechere alafanechere temporarily deployed to more-secrets April 28, 2022 10:41 Inactive
@alafanechere alafanechere temporarily deployed to more-secrets April 28, 2022 10:41 Inactive
Copy link
Contributor

@alafanechere alafanechere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for spotting this. How did you find it? How is this a blocker for you (just for my own curiosity)?
I'm running the CI to test on a non forked branch and will get back for merge.

@alafanechere
Copy link
Contributor

I confirm that CI tests are passing. I successfully created a connection from a UI with a build on this branch.

@Ohcui
Copy link
Contributor Author

Ohcui commented Apr 29, 2022

Thank you for spotting this. How did you find it? How is this a blocker for you (just for my own curiosity)? I'm running the CI to test on a non forked branch and will get back for merge.

Yes, I have TypeScript definitions for AirByte API which is generated from this config file, and this issue leads to a wrong definition which is:

type WebBackendConnectionCreate = {
  // properties
} & {
  connection: unknown
}

and the expected is:

type WebBackendConnectionCreate = {
  // properties
}

For me, I have to maintiain a patch memo and remind my self every time when the definition is generated. It is really a head burden.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api Related to the api area/platform issues related to the platform community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants