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 un-used update/delete custom connector api #20034

Merged
merged 3 commits into from
Dec 5, 2022
Merged

Conversation

xiaohansong
Copy link
Contributor

@xiaohansong xiaohansong commented Dec 2, 2022

What

#19670
https://github.com/airbytehq/airbyte-cloud/issues/3642

remove un-used update/delete custom connector api and create connector api

@octavia-squidington-iv octavia-squidington-iv added area/api Related to the api area/documentation Improvements or additions to documentation area/platform issues related to the platform area/server labels Dec 2, 2022
@xiaohansong xiaohansong temporarily deployed to more-secrets December 2, 2022 19:53 Inactive
@pmossman
Copy link
Contributor

pmossman commented Dec 2, 2022

@marcelopio I know you mentioned working on CLI custom connectors in an issue comment earlier, can you verify that this PR is ok with your changes as well?

Copy link
Contributor

@pmossman pmossman left a comment

Choose a reason for hiding this comment

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

Looks good to me, assuming builds pass!

@xiaohansong
Copy link
Contributor Author

@pmossman Took another pass and I think it should be straightforward to remove the creation in the same PR - PTAL if you got a chance!

And for @marcelopio's PR - I commented in your PR, the update path needs to be changed

@xiaohansong xiaohansong temporarily deployed to more-secrets December 2, 2022 20:25 Inactive
@marcelopio
Copy link
Contributor

@marcelopio I know you mentioned working on CLI custom connectors in an issue comment earlier, can you verify that this PR is ok with your changes as well?

It is ok for me. Actually this solves a doubt I was having if I should use custom or not.

Now I just have the question if I should manage connectors version for non custom connectors, but that is to be discussed on my PR. Thanks!

@pmossman
Copy link
Contributor

pmossman commented Dec 2, 2022

@xiaohansong looks like there may be some broken tests in the build after removing those endpoints, maybe some tests need to be converted to the createCustom version of the endpoint?

@josephkmh
Copy link
Contributor

Just confirming that we are not using any of these endpoints on the frontend anymore 👍

@xiaohansong xiaohansong temporarily deployed to more-secrets December 5, 2022 17:32 Inactive
@xiaohansong xiaohansong merged commit 2a16285 into master Dec 5, 2022
@xiaohansong xiaohansong deleted the xiaohan/cusmerge branch December 5, 2022 18:12
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/documentation Improvements or additions to documentation area/platform issues related to the platform area/server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants