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

Add OpenAPIV3 schema validation to the HiveTable CRD. #887

Closed
wants to merge 2 commits into from
Closed

Add OpenAPIV3 schema validation to the HiveTable CRD. #887

wants to merge 2 commits into from

Conversation

timflannagan
Copy link
Contributor

@timflannagan timflannagan commented Aug 8, 2019 •
edited
Loading

Overview

This would add schema validation to the HiveTable CRD. I verified that you need at the minimum, the tableName, columns, and databaseName for a HiveTable resource to turn into a valid table in Presto.

To-Do:

  • Add oneOf to top-level spec.properties to enforce certain configurations.
  • Update hivetables.md to clarify what fields are required, and optional.
  • Update hivetables.md and add more examples.

@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Aug 8, 2019
@timflannagan
Copy link
Contributor Author

/retest

@timflannagan timflannagan changed the title WIP: Add OpenAPIV3 schema validation to the HiveTable CRD. Add OpenAPIV3 schema validation to the HiveTable CRD. Aug 9, 2019
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 9, 2019
@timflannagan timflannagan force-pushed the hivetable-schema-validation branch 2 times, most recently from 473c1d8 to 3cd9bc0 Compare August 10, 2019 16:46
@timflannagan timflannagan force-pushed the hivetable-schema-validation branch 2 times, most recently from b2cb216 to 29d310d Compare August 12, 2019 17:22
@timflannagan
Copy link
Contributor Author

The oneOf clause isn't handling all invalid configurations.
/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 12, 2019
@timflannagan
Copy link
Contributor Author

/hold remove

@timflannagan
Copy link
Contributor Author

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 12, 2019
@chancez
Copy link
Contributor

chancez commented Aug 13, 2019

/lgtm
/approve

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 13, 2019
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chancez, EmilyM1, timflannagan1

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 13, 2019
@timflannagan
Copy link
Contributor Author

the prow jobs have been frozen in pending for 15 hours
/retest

@timflannagan
Copy link
Contributor Author

/retest

4 similar comments
@timflannagan
Copy link
Contributor Author

/retest

@timflannagan
Copy link
Contributor Author

/retest

@chancez
Copy link
Contributor

chancez commented Aug 14, 2019

/retest

@timflannagan
Copy link
Contributor Author

/retest

@timflannagan timflannagan reopened this Aug 14, 2019
@timflannagan
Documentation: Update descriptions, organization of fields, and add m…
676e75c 
…ore HiveTable examples.
@timflannagan
charts,manifests: Add OpenAPIV3 schema validation to the HiveTable CRD.
bbdb029 
Other notable changes in this commit:

1. charts,manifests: Add 'nullable: true' to the properties array.

2. charts,manifests: Enforce certain sub-schema configurations.
@timflannagan timflannagan reopened this Aug 14, 2019
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Aug 14, 2019
@openshift-ci-robot
Copy link

New changes are detected. LGTM label has been removed.

@openshift-ci-robot
Copy link

@timflannagan1: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/prow/unit bbdb029 link /test unit
ci/prow/metering-e2e-aws bbdb029 link /test metering-e2e-aws
ci/prow/verify bbdb029 link /test verify
ci/prow/images bbdb029 link /test images

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@timflannagan timflannagan deleted the hivetable-schema-validation branch August 14, 2019 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chancez chancez chancez left review comments

@EmilyM1 EmilyM1 EmilyM1 approved these changes

@bentito bentito Awaiting requested review from bentito

Assignees

@chancez chancez

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@timflannagan @chancez @openshift-ci-robot @EmilyM1