-
Notifications
You must be signed in to change notification settings - Fork 350
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
feat: schema for apisixroute #345
Conversation
Codecov Report
@@ Coverage Diff @@
## master #345 +/- ##
==========================================
- Coverage 43.84% 43.50% -0.35%
==========================================
Files 40 39 -1
Lines 3435 3455 +20
==========================================
- Hits 1506 1503 -3
- Misses 1761 1781 +20
- Partials 168 171 +3
Continue to review full report at Codecov.
|
properties: | ||
scope: | ||
type: string | ||
enum: ["Header", "Query", "Cookie", "Path"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should enums be sorted consistently the same way in each file ?
enum: ["Header", "Query", "Cookie", "Path"] | |
enum: ["Cookie", "Header", "Path", "Query"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point.
minItems: 1 | ||
items: | ||
type: string | ||
enum: ["GET", "POST", "PUT", "DELETE", "PATCH", "HEAD", "OPTIONS", "CONNECT", "TRACE"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whilst "GET" and "POST" are probably the most used or probably the most well known, should an enum array or list have a sort order or be random ?
enum: ["GET", "POST", "PUT", "DELETE", "PATCH", "HEAD", "OPTIONS", "CONNECT", "TRACE"] | |
enum: ["CONNECT", "DELETE", "GET", "HEAD", "OPTIONS", "PATCH", "POST", "PUT", "TRACE"] |
- scope | ||
op: | ||
type: string | ||
enum: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we sort enums ? Or at least sort some part of the enums ?
- LessThan | ||
- In | ||
- NotIn | ||
- RegexMatch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So for this enum maybe keep the top part with the custom order and then for the bottom part sort it ?
- RegexMatch | |
- RegexMatch | |
- RegexMatchCaseInsensitive | |
- RegexNotMatch | |
- RegexNotMatchCaseInsensitive |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I grouped them by their meanings.
items: | ||
type: string | ||
oneOf: | ||
- required: ["subject", "op", "value"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- required: ["subject", "op", "value"] | |
- required: ["op", "subject", "value"] |
type: string | ||
oneOf: | ||
- required: ["subject", "op", "value"] | ||
- required: ["subject", "op", "set"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- required: ["subject", "op", "set"] | |
- required: ["op", "set", "subject"] |
items: | ||
type: object | ||
oneOf: | ||
- required: ["name", "match", "backend"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- required: ["name", "match", "backend"] | |
- required: ["backend", "match", "name"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I grouped them by their meanings.
type: object | ||
oneOf: | ||
- required: ["name", "match", "backend"] | ||
- required: ["name", "match", "backends"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- required: ["name", "match", "backends"] | |
- required: ["backends", "match", "name"] |
@jbampton Fields like |
minItems: 1 | ||
items: | ||
type: string | ||
pattern: "^/" # start with "/" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pattern: "^/" # start with "/" | |
pattern: "^\/[a-zA-Z0-9\-._~%!$&'()+,;=:@/]*\*?$" |
Please answer these questions before submitting a pull request
Why submit this pull request?
Bugfix
New feature provided
Improve performance
Backport patches
Related issues
Bugfix
Description
How to fix?
New feature or improvement
Backport patches
Why need to backport?
Source branch
Related commits and pull requests
Target branch