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.

Sign up for GitHub

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

Jump to bottom

deprecate import / export endpoints #16175

Merged
merged 3 commits into from
Sep 6, 2022
Merged

deprecate import / export endpoints #16175

merged 3 commits into from
Sep 6, 2022

Conversation

cgardens
Copy link
Contributor

What

  • These are no longer used for migrations and it is easy to use them in a way that will shoot the user in the foot by accident.

Follow up

  • This PR just kills the endpoints. A follow up will delete the handlers associated with it as they are no longer needed and it will remove a lot of tricky code.

@github-actions github-actions bot added area/api Related to the api area/documentation Improvements or additions to documentation area/platform issues related to the platform area/server labels Aug 31, 2022
@cgardens cgardens temporarily deployed to more-secrets August 31, 2022 17:16 Inactive
sherifnada
sherifnada approved these changes Aug 31, 2022
View reviewed changes
Copy link
Contributor

@sherifnada sherifnada left a comment

Choose a reason for hiding this comment

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

is there a corresponding UI change?

davinchia
davinchia approved these changes Aug 31, 2022
View reviewed changes
Copy link
Contributor

@davinchia davinchia left a comment •
edited

Choose a reason for hiding this comment

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

Yeah, I think we might need to remove these from the frontend.

cc @timroes
Screenshot 2022-08-31 at 10 38 01 AM

We can do so in a follow up PR too. As long as we do it before we publish the next release.

Looks like this is the only usage for now, so I think the entire ArchiveHandler can be removed too. We can also do that in a follow up PR. Not sure if we want to. Mainly calling out.

@davinchia
Copy link
Contributor

Screenshot 2022-08-31 at 10 40 43 AM

Not sure why my screenshot didn't upload

@davinchia
Copy link
Contributor

Looks like this is failing the Octavia CLI build - @alafanechere are the import/export routes needed for the CLI?

@alafanechere
Copy link
Contributor

Looks like this is failing the Octavia CLI build - @alafanechere are the import/export routes needed for the CLI?

Nope, but CLI builds depends on airbyte-webapp which is failing:

Execution failed for task ':airbyte-webapp:npm_run_build'.

@evantahler
Copy link
Contributor

I'm 👍 on removing dangerous old endpoints, but we probably should do /all/ the removing in one PR... which is probably needed to get the build to pass. We don't have a great track record of remembering to clean up aftewrods...

I can help @cgardens and do the FE cleanup.

@evantahler evantahler mentioned this pull request Aug 31, 2022
Closed
@evantahler
Copy link
Contributor

evantahler commented Aug 31, 2022 •
edited

#16186 has the UI updates and the build is green

@cgardens cgardens requested a review from a team as a code owner August 31, 2022 23:17
@github-actions github-actions bot added the area/frontend Related to the Airbyte webapp label Aug 31, 2022
@cgardens cgardens temporarily deployed to more-secrets August 31, 2022 23:20 Inactive
@cgardens
Copy link
Contributor Author

Screen Shot 2022-08-31 at 4 38 24 PM

Updated FE code to remove dep on these API endpoints. @timroes

@cgardens cgardens requested a review from timroes August 31, 2022 23:39
@evantahler
Copy link
Contributor

evantahler commented Aug 31, 2022 •
edited

There are a few other things you can remove that aren't in use after removing the config upload and download

  • the react-dropzone dependency in package.json
  • the dropZone locale strings in airbyte-webapp/src/locales/en.json
  • the components/FileDropZone component

@cgardens cgardens temporarily deployed to more-secrets August 31, 2022 23:56 Inactive
@cgardens cgardens temporarily deployed to more-secrets September 1, 2022 02:27 Inactive
@krishnaglick
Copy link
Contributor

I've been using this feature locally to easily "reset" my connections so I can test onboarding and clean things up for local end-to-end testing. Since this seems to be being removed in its totality is there an easy way, maybe with Octavia CLI, to clean things out without truncating some tables?

@cgardens
Copy link
Contributor Author

cgardens commented Sep 6, 2022

@krishnaglick yeah. I think octavia CLI is your best bet. You should be able to get the same behavior.

@cgardens cgardens merged commit ce201bc into master Sep 6, 2022
@cgardens cgardens deleted the cgardens/deprecate-import-export branch September 6, 2022 16:34
@octavia-squidington-iii octavia-squidington-iii mentioned this pull request Sep 12, 2022
Merged
robbinhan pushed a commit to robbinhan/airbyte that referenced this pull request Sep 29, 2022
@cgardens @robbinhan
deprecate import / export endpoints (airbytehq#16175)
4732d22 
* deprecate import / export endpoints

* remove fe deps on import / export

* additional fe clean up
jhammarstedt pushed a commit to jhammarstedt/airbyte that referenced this pull request Oct 31, 2022
@cgardens @jhammarstedt
deprecate import / export endpoints (airbytehq#16175)
dd50fd8 
* deprecate import / export endpoints

* remove fe deps on import / export

* additional fe clean up
@andresbravog
Copy link
Contributor

Question here, we have been using this feature for both store daily backups of the Airbyte configuration and state being able to back to a past state for all connections if something critical happens.

My understanding is that Octavia's client does not extract/store/modify connection state and cursors. Is that correct?

@cgardens
Copy link
Contributor Author

yeah. that's correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@edmundito edmundito edmundito approved these changes

@evantahler evantahler evantahler approved these changes

@sherifnada sherifnada sherifnada approved these changes

@davinchia davinchia davinchia approved these changes

@benmoriceau benmoriceau Awaiting requested review from benmoriceau

@timroes timroes Awaiting requested review from timroes

Assignees
No one assigned
Labels
area/api Related to the api area/documentation Improvements or additions to documentation area/frontend Related to the Airbyte webapp area/platform issues related to the platform area/server
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@cgardens @davinchia @alafanechere @evantahler @krishnaglick @andresbravog @edmundito @sherifnada