Skip to content

feat: support external objects in schema tree #485

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
Merged
merged 2 commits into from
Jul 27, 2023
Merged

feat: support external objects in schema tree #485

merged 2 commits into from
Jul 27, 2023

Conversation

artemmufazalov
Copy link
Member

@artemmufazalov artemmufazalov commented Jul 25, 2023 •
edited
Loading

Support External Table and External Data Source in schema tree:

  • Objects icons
  • Objects context actions - "copy" for Data Source, "copy", "preview" and "select" for Table
  • Preview for External Table
  • Overview and Acl summary tabs for both, Schema tab without key column for External Table
  • Info tab with title in Diagnostics (additional fields will be added next)

All SQL request to External Data Source and External Table work only with YQL - QueryService query type

Screen Shot 2023-07-25 at 12 06 29

@artemmufazalov artemmufazalov requested a review from Raubzeug July 25, 2023 09:10
@artemmufazalov artemmufazalov marked this pull request as ready for review July 25, 2023 09:15
@Raubzeug
Copy link
Contributor

If SQL request to External Data Source and External Table work only with YQL - QueryService query type we should give user any clue. I suggest two things:

  1. if required query type is not enabled in settings, we should disable Run button and add a tooltip with info.
  2. if query type is available, we should select it and disable query type selector (maybe help tooltip will be useful too).

@Raubzeug
Copy link
Contributor

IMAGE 2023-07-26 18:03:30

Please check this error.

@artemmufazalov artemmufazalov requested a review from krosy1337 July 26, 2023 15:23
@artemmufazalov
Copy link
Member Author

If SQL request to External Data Source and External Table work only with YQL - QueryService query type we should give user any clue. I suggest two things:

  1. if required query type is not enabled in settings, we should disable Run button and add a tooltip with info.
  2. if query type is available, we should select it and disable query type selector (maybe help tooltip will be useful too).

I don't think it's correct solution due to two reasons:

  1. QueryEditor works in the context of database, not the object selected in schema tree. So it won't be correct to disable controls or other query modes
  2. Some actions to external object works with YQL-QueryService type (select), other - with YQL Script (create, drop)

What can be done:

  1. When some action is chosen in context menu, select required QueryMode
  2. If required QueryMode cannot be selected (not enabled in settings), show a toast with an error and request to turn on the setting

Anyway, I'd prefer to do it in a separate PR

Raubzeug
Raubzeug approved these changes Jul 27, 2023
View reviewed changes
src/containers/Tenant/utils/schemaActions.ts
@@ -83,23 +83,25 @@ export const getActions =
(path: string, type: NavigationTreeNodeType) => {
const actions = bindActions(path, dispatch, setActivePath);
const copyItem = {text: 'Copy path', action: actions.copyPath};
const openPreview = {text: 'Open preview', action: actions.openPreview};
Copy link
Contributor

Choose a reason for hiding this comment

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

i18n?

Copy link
Member Author

Choose a reason for hiding this comment

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

I haven't figured out yet how correctly translate "Select query", "Upsert Query" and "Alter table", so just let it be

@artemmufazalov artemmufazalov merged commit cf96f9a into main Jul 27, 2023
@artemmufazalov artemmufazalov deleted the external-objects branch July 27, 2023 13:29
@ydb-platform-bot ydb-platform-bot mentioned this pull request Jul 27, 2023
Merged
@artemmufazalov artemmufazalov mentioned this pull request Jul 31, 2023
Merged
This was referenced Jan 31, 2024
Closed
Closed
Closed
Merged
@ydb-platform-bot ydb-platform-bot mentioned this pull request Jun 14, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Raubzeug Raubzeug Raubzeug approved these changes

@krosy1337 krosy1337 Awaiting requested review from krosy1337

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@artemmufazalov @Raubzeug