You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "charlesvhe (via GitHub)" <gi...@apache.org> on 2023/03/09 13:51:30 UTC
[GitHub] [apisix] charlesvhe opened a new pull request, #9043: add nacos metadata suport, for customize balancer
charlesvhe opened a new pull request, #9043:
URL: https://github.com/apache/apisix/pull/9043
### Description
In order to customize the nacos-based balancer, needs nacos instance metadata. So metadata needs to be added to the node information of apisix nacos discovery
Fixes #5706 request help: How to route according to the configuration of nacos metadata?
### Checklist
- [ X ] I have explained the need for this PR and the problem it solves
- [ X ] I have explained the changes or the new features added to this PR
- [ X ] I have added tests corresponding to this change
- [ X ] I have updated the documentation to reflect this change
- [ X ] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first)
<!--
Note
1. Mark the PR as draft until it's ready to be reviewed.
2. Always add/update tests for any changes unless you have a good reason.
3. Always update the documentation to reflect the changes made in the PR.
4. Make a new commit to resolve conversations instead of `push -f`.
5. To resolve merge conflicts, merge master instead of rebasing.
6. Use "request review" to notify the reviewer after making changes.
7. Only a reviewer can mark a conversation as resolved.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] charlesvhe commented on a diff in pull request #9043: feat: add nacos metadata suport, for customize balancer
Posted by "charlesvhe (via GitHub)" <gi...@apache.org>.
charlesvhe commented on code in PR #9043:
URL: https://github.com/apache/apisix/pull/9043#discussion_r1132119439
##########
apisix/discovery/nacos/init.lua:
##########
@@ -315,6 +315,7 @@ local function fetch_full_registry(premature)
host = host.ip,
port = host.port,
weight = host.weight or default_weight,
+ metadata = host.metadata or {},
Review Comment:
In fact, nacos currently returns an empty json object when metadata is not configured, which corresponds to an empty table in lua. In order to keep this feature stable, this is a failback mechanism. Prevent future destructive changes to existing apisix code if metadata does not return or null is returned by nacos next release
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] charlesvhe closed pull request #9043: feat: add nacos metadata suport, for customize balancer
Posted by "charlesvhe (via GitHub)" <gi...@apache.org>.
charlesvhe closed pull request #9043: feat: add nacos metadata suport, for customize balancer
URL: https://github.com/apache/apisix/pull/9043
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] monkeyDluffy6017 commented on a diff in pull request #9043: feat: add nacos metadata suport, for customize balancer
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9043:
URL: https://github.com/apache/apisix/pull/9043#discussion_r1131866661
##########
apisix/discovery/nacos/init.lua:
##########
@@ -315,6 +315,7 @@ local function fetch_full_registry(premature)
host = host.ip,
port = host.port,
weight = host.weight or default_weight,
+ metadata = host.metadata or {},
Review Comment:
Why the default value is an empty table, is nil more appropriate?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] monkeyDluffy6017 commented on pull request #9043: feat: add nacos metadata suport, for customize balancer
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on PR #9043:
URL: https://github.com/apache/apisix/pull/9043#issuecomment-1463092195
We need a test case in apisix/t/discovery/nacos.t
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] charlesvhe commented on pull request #9043: feat: add nacos metadata suport, for customize balancer
Posted by "charlesvhe (via GitHub)" <gi...@apache.org>.
charlesvhe commented on PR #9043:
URL: https://github.com/apache/apisix/pull/9043#issuecomment-1463617153
>
I have read the existing test case and currently no component uses this metadata for testing. I will close this PR and submit another PR later for a node_label_balancer component together
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org