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

fix: fix the upstream timeout unit #1580

Merged
merged 4 commits into from Mar 15, 2021
Merged

fix: fix the upstream timeout unit #1580

merged 4 commits into from Mar 15, 2021

Conversation

stu01509
Copy link
Contributor

Please answer these questions before submitting a pull request


Bugfix

  • Description

  • How to fix?
    Change the upstream timeout unit to s.
    image

@codecov-io
Copy link

codecov-io commented Mar 12, 2021

Codecov Report

Merging #1580 (11dcccc) into master (e22de29) will increase coverage by 3.42%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1580      +/-   ##
==========================================
+ Coverage   69.01%   72.44%   +3.42%     
==========================================
  Files         132       86      -46     
  Lines        5374     2355    -3019     
  Branches      592      592              
==========================================
- Hits         3709     1706    -2003     
+ Misses       1417      649     -768     
+ Partials      248        0     -248     
Flag Coverage Δ
backend-e2e-test-ginkgo ?
backend-unit-test ?
frontend-e2e-test 72.44% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
web/src/components/Upstream/constant.ts 100.00% <ø> (ø)
web/src/components/Upstream/UpstreamForm.tsx 62.01% <100.00%> (ø)
api/internal/core/entity/entity.go
api/internal/utils/version.go
api/internal/utils/utils.go
api/internal/handler/service/service.go
api/internal/handler/label/label.go
api/internal/core/store/validate.go
api/internal/utils/pid.go
api/internal/handler/global_rule/global_rule.go
... and 38 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e22de29...11dcccc. Read the comment docs.

Copy link
Member

@juzhiyuan juzhiyuan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, and would like to update those 3 fields' value to 6? for they are in seconds 🤔

https://github.com/stu01509/apisix-dashboard/blob/fix-%231565-use-seconds/web/src/components/Upstream/constant.ts#L22-L24

@stu01509
Copy link
Contributor Author

LGTM, and would like to update those 3 fields' value to 6? for they are in seconds 🤔

https://github.com/stu01509/apisix-dashboard/blob/fix-%231565-use-seconds/web/src/components/Upstream/constant.ts#L22-L24

Agree, So I update the commit.

@stu01509 stu01509 requested a review from juzhiyuan March 13, 2021 04:41
@stu01509
Copy link
Contributor Author

Hi @liuxiran and @nic-chen

Thanks for point out, so should I fix the E2E test error or waiting for @nic-chen to solve it?

@liuxiran
Copy link
Contributor

Hi @liuxiran and @nic-chen

Thanks for point out, so should I fix the E2E test error or waiting for @nic-chen to solve it?

you can try to fix the e2e error in route/import_export_route.spec.js first😊 @stu01509

@nic-chen
Copy link
Member

thanks @liuxiran

@stu01509
Copy link
Contributor Author

Hi @liuxiran and @nic-chen

Update the commit, fix the import_export_route.spec.js compare file error.

@nic-chen
Copy link
Member

wait for #1594

@nic-chen
Copy link
Member

hi, please sync codes from branch master.

@stu01509
Copy link
Contributor Author

hi, please sync codes from branch master.

Done!

@LiteSun LiteSun merged commit 40d1dab into apache:master Mar 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The upstream timeout unit is seconds instead of milliseconds
6 participants