-
Notifications
You must be signed in to change notification settings - Fork 534
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: basic support Apache APISIX 2.9 #2117
Conversation
✔️ Deploy Preview for apisix-dashboard ready! 🔨 Explore the source changes: 5e26fc4 🔍 Inspect the deploy log: https://app.netlify.com/sites/apisix-dashboard/deploys/61321d8d9b94d0000725d6b7 😎 Browse the preview: https://deploy-preview-2117--apisix-dashboard.netlify.app |
52bc53b
to
1837ca7
Compare
Codecov Report
@@ Coverage Diff @@
## master #2117 +/- ##
==========================================
+ Coverage 67.31% 67.48% +0.16%
==========================================
Files 124 126 +2
Lines 3283 3294 +11
Branches 800 802 +2
==========================================
+ Hits 2210 2223 +13
+ Misses 1073 1071 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
3923974
to
567cce5
Compare
567cce5
to
6c7184f
Compare
b07b192
to
36e2761
Compare
36e2761
to
86b5ba7
Compare
@nic-chen please help to review, thanks a lot |
ExpectStatus: http.StatusOK, | ||
}) | ||
}) | ||
}) |
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.
Need dp test, please refer to APISIX related test
UnHealthy UnHealthy `json:"unhealthy,omitempty"` | ||
ReqHeaders []string `json:"req_headers,omitempty"` | ||
Type string `json:"type,omitempty"` | ||
Timeout TimeoutValue `json:"timeout,omitempty"` |
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.
Why change this to float32?
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.
There are two "number" definitions in jsonschema, one is an integer, the other is number, where number runs to store float values. Such as 5.5
Therefore, a float value is used here to store.
In addition, only map[string]interface{} was used to store the value before, and there was ambiguity in the data type. Crash may occur during runtime. This is also the reason for this modification.(add a Timeout struct)
@@ -97,10 +97,11 @@ type Route struct { | |||
} | |||
|
|||
// --- structures for upstream start --- | |||
type TimeoutValue float32 |
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.
why
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.
There are two "number" definitions in jsonschema, one is an integer
, the other is number
, where number
runs to store float values. Such as 5.5
Therefore, a float value is used here to store.
In addition, only map[string]interface{} was used to store the value before, and there was ambiguity in the data type. Crash may occur during runtime. This is also the reason for this modification.(add a Timeout struct)
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.
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 do we support timeout 5.5s?
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.
Why submit this pull request?
What changes will this PR take into?
Provide basic support for Apache APISIX 2.9. Includes changes to the upstream property definition.
Checklist: