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

SAT: check oneOf common property has only const keyword, no default and enum keywords #11703

Closed
grubberr opened this issue Apr 4, 2022 · 3 comments · Fixed by #11704
Closed
Assignees
Labels
type/bug Something isn't working

Comments

@grubberr
Copy link
Contributor

grubberr commented Apr 4, 2022

We need to improve SAT test_oneof_usage to check that common oneOf property looks like this:

"option_title": {
    "type": "string",
    "const": "OAuth Credentials",
    "order": 0
}

only const keyword
NO MORE default and enum keywords.

old value

"option_title": {
    "type": "string",
    "const": "OAuth Credentials",
    "enum": ["OAuth Credentials"],
    "default": "OAuth Credentials",
    "order": 0
}
@grubberr grubberr added type/bug Something isn't working needs-triage labels Apr 4, 2022
@grubberr grubberr changed the title SAT: improve test_oneof_usage common_prop const equal default SAT: improve test_oneof_usage common_prop const equals default Apr 4, 2022
@grubberr grubberr self-assigned this Apr 4, 2022
@grubberr grubberr moved this to Implementation in progress in GL Roadmap Apr 4, 2022
@sherifnada
Copy link
Contributor

@grubberr cant we just use const keyword always instead of adding the enum and default keywords?

@grubberr
Copy link
Contributor Author

grubberr commented Apr 5, 2022

@sherifnada I just checked, const only keyword also can work!

I see we have 2 option:

  1. add new SAT test which makes sure we have only const and don't have enum and don't have default keywords

or

  1. also check (1) but to be backward compatible if not (1) also check if we have all 3 keywords check that const == default and const in enum

What is better (a) or (b) ? What do you think ?

@sherifnada
Copy link
Contributor

I think 1 is better, because it makes the iface simpler for the user

@oustynova oustynova moved this from Implementation in progress to In review (internal) in GL Roadmap Apr 6, 2022
@grubberr grubberr changed the title SAT: improve test_oneof_usage common_prop const equals default SAT: check oneOf common property has only const keyword, no default and enum keywords Apr 6, 2022
@grubberr grubberr moved this from In review (internal) to In review (Airbyte) in GL Roadmap Apr 6, 2022
@grubberr grubberr moved this from In review (Airbyte) to Done in GL Roadmap Apr 7, 2022
@lazebnyi lazebnyi removed this from GL Roadmap Apr 7, 2022
@lazebnyi lazebnyi moved this to Done in GL Roadmap Apr 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Something isn't working
Projects
No open projects
Archived in project
2 participants