-
Notifications
You must be signed in to change notification settings - Fork 542
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
fix: CI failed #1594
fix: CI failed #1594
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1594 +/- ##
==========================================
+ Coverage 69.01% 70.05% +1.03%
==========================================
Files 132 46 -86
Lines 5374 3019 -2355
Branches 592 0 -592
==========================================
- Hits 3709 2115 -1594
+ Misses 1417 659 -758
+ Partials 248 245 -3
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
@@ -81,7 +81,7 @@ type Route struct { | |||
Hosts []string `json:"hosts,omitempty"` | |||
RemoteAddr string `json:"remote_addr,omitempty"` | |||
RemoteAddrs []string `json:"remote_addrs,omitempty"` | |||
Vars interface{} `json:"vars,omitempty"` | |||
Vars []interface{} `json:"vars,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 should the data format be modified here? Whether it has influence on the existing FE and BE logic.
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.
The same question as @Jaycean.
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.
@Jaycean please take a look at Bugfix Description
, I wrote the reason there.
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.
Sorry, my question. I didn't read the description carefully.
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.
cc @nic-chen
FE CI failed:https://github.com/apache/apisix-dashboard/pull/1594/checks?check_run_id=2110905208
Remove the corresponding data: "x-apimix-vars": []
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.
This line of code also needs to be removed:
"x-apisix-vars": [] |
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.
LGTM
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.
LGTM for fe e2e changs
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
Vars
in structRoute
uses interface{} as the type, which makes it impossible to omit an empty array.As a result, after APISIX uses the new version of
lua-resty-radixtree
, The routes with empty arrayvars
cannot be matcheduses []interface{} instead. and also fix some small potential unstable issues