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

feat(upstream): add upstream priority field #2271

Merged
merged 10 commits into from
Jan 1, 2022
Merged

feat(upstream): add upstream priority field #2271

merged 10 commits into from
Jan 1, 2022

Conversation

zaunist
Copy link
Contributor

@zaunist zaunist commented Dec 29, 2021

Please answer these questions before submitting a pull request, or your PR will get closed.

Why submit this pull request?

  • Bugfix
  • New feature provided
  • Improve performance
  • Backport patches

What changes will this PR take into?

Add upstream priority support.

Related issues

fix #2192

Checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

@zaunist
Copy link
Contributor Author

zaunist commented Dec 29, 2021

support priority now

image

@codecov-commenter
Copy link

codecov-commenter commented Dec 29, 2021

Codecov Report

Merging #2271 (3f0fac4) into master (25ac15a) will increase coverage by 1.72%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2271      +/-   ##
==========================================
+ Coverage   68.07%   69.80%   +1.72%     
==========================================
  Files         127      184      +57     
  Lines        3370     7275    +3905     
  Branches      829      829              
==========================================
+ Hits         2294     5078    +2784     
- Misses       1076     1903     +827     
- Partials        0      294     +294     
Flag Coverage Δ
backend-e2e-test 37.25% <100.00%> (?)
backend-e2e-test-ginkgo 59.38% <100.00%> (?)
backend-unit-test 49.22% <100.00%> (?)
frontend-e2e-test 67.95% <ø> (-0.12%) ⬇️

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

Impacted Files Coverage Δ
api/internal/core/entity/entity.go 90.90% <ø> (ø)
api/internal/core/entity/format.go 67.30% <100.00%> (ø)
web/src/components/Plugin/PluginDetail.tsx 67.05% <0.00%> (-3.47%) ⬇️
api/internal/log/log.go 60.00% <0.00%> (ø)
api/internal/utils/utils.go 73.00% <0.00%> (ø)
api/internal/utils/closer.go 66.66% <0.00%> (ø)
api/internal/handler/migrate/migrate.go 59.67% <0.00%> (ø)
api/internal/core/migrate/migrate.go 76.19% <0.00%> (ø)
api/internal/core/storage/storage_mock.go 0.00% <0.00%> (ø)
api/internal/handler/schema/plugin.go 100.00% <0.00%> (ø)
... and 51 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 25ac15a...3f0fac4. Read the comment docs.

@zaunist zaunist changed the title fix(upstream): add upstream priority feat(upstream): add upstream priority field Dec 30, 2021
@zaunist zaunist marked this pull request as ready for review December 30, 2021 03:32
@zaunist
Copy link
Contributor Author

zaunist commented Dec 30, 2021

Hi, community. I have add a field to node, and I already update backend code and E2E test case, but this change will effect frontend, so I need someone to help me to update frontend code. @juzhiyuan @guoqqqi

@juzhiyuan
Copy link
Member

Hi, community. I have add a field to node, and I already update backend code and E2E test case, but this change will effect frontend, so I need someone to help me to update frontend code. @juzhiyuan @guoqqqi

Hi @zaunist, just contacted with @guoqqqi, he will help you to do this :)

@zaunist
Copy link
Contributor Author

zaunist commented Dec 30, 2021

For support FQDN #2080, we can't support edit priority on page, only support edit in editor.

image

@zaunist
Copy link
Contributor Author

zaunist commented Dec 30, 2021

cc @nic-chen @bzp2010 @starsz

@guoqqqi
Copy link
Member

guoqqqi commented Dec 30, 2021

need to fix the e2e test

@zaunist
Copy link
Contributor Author

zaunist commented Dec 30, 2021

cc @guoqqqi

@guoqqqi
Copy link
Member

guoqqqi commented Dec 31, 2021

It looks like we can now only configure the priority field via the editor, and we may need to update the node configuration of the upstream configuration form:
How about adding a type selection for the configuration upstream form node? For example, when selecting the FQDN type, only localhost and weight are displayed; otherwise, all are displayed.

Of course, we can do it in another PR if we need to. What do others think of it? 😄

@zaunist zaunist requested a review from starsz December 31, 2021 06:04
api/internal/core/store/validate.go Outdated Show resolved Hide resolved
@juxianzheng
Copy link
Contributor

看来我们现在只能通过编辑器配置priority字段了,可能需要更新上游配置表单的节点配置: 为配置上游表单节点添加一个类型选择怎么样?比如选择FQDN类型时,只显示localhost和权重;否则,全部显示。

当然,如果需要,我们可以在另一个 PR 中进行。其他人怎么看?😄

查看我们现在可以通过编辑器配置优先级字段了,可能需要更新配置表单的节点配置: 为配置启动表单节点添加一个类型选择吗?比如选择FQDN类型时,只显示本地主机和权限重;否则,全部显示。

当然,如果需要,我们可以在另一个公关中进行。其他人怎么看?😄

I think it can

@juzhiyuan juzhiyuan merged commit bec322a into apache:master Jan 1, 2022
@zaunist zaunist deleted the feat/upstream_node branch January 1, 2022 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
8 participants