Skip to content

Make key fields mandatory for adding/updating views in CLI#1368

Merged
abhishekkumams merged 28 commits intomainfrom
dev/abhishekkuma/making_keyFields_mandatory_forp_views_in_cli
Mar 31, 2023
Merged

Make key fields mandatory for adding/updating views in CLI#1368
abhishekkumams merged 28 commits intomainfrom
dev/abhishekkuma/making_keyFields_mandatory_forp_views_in_cli

Conversation

@abhishekkumams
Copy link
Contributor

Why make this change?

What is this change?

  • Added a validation check in CLI to mandate key-fields when adding views.

How was this tested?

  • Unit Tests

Sample Request(s)

Adding a view without KeyFields

image

Adding a view with KeyFields

image

Updating a table to view without KeyFields

image

Updating a table to view with KeyFields

image

NOTE:

Engine will continue to support config with views that do not have key-fields provided in the config. This Validation is only applicable when adding/updating views through CLI.

@abhishekkumams abhishekkumams changed the title Dev/abhishekkuma/making key fields mandatory forp views in cli Make key fields mandatory for adding/updating views in CLI Mar 28, 2023
Copy link
Contributor

@seantleonard seantleonard 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 so far, pending tests/documentation to add before merge.

Copy link
Contributor

@seantleonard seantleonard left a comment

Choose a reason for hiding this comment

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

need additional result checking for GraphQL request in your mssql view test. otherwise looks good. thank you for addressing comments.

Copy link
Collaborator

@Aniruddh25 Aniruddh25 left a comment

Choose a reason for hiding this comment

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

LGTM, after the graphql response check

@abhishekkumams abhishekkumams merged commit 559e0da into main Mar 31, 2023
@abhishekkumams abhishekkumams deleted the dev/abhishekkuma/making_keyFields_mandatory_forp_views_in_cli branch March 31, 2023 06:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make Key-fields necessary for Views

4 participants