Develop v2 for config_client #183
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
CZJCC
reviewed
Oct 11, 2024
| @@ -2,6 +2,9 @@ | |||
|
|
|||
| from v2.nacos.transport.model.rpc_response import Response | |||
|
|
|||
| class NotifySubscriberResponse(Response): | |||
| def get_response_type(self) -> str: | |||
| return "ConnectResetResponse" | |||
test/client_v2_test.py
Outdated
| @@ -31,4 +31,4 @@ async def test_naming_register(self): | |||
| print("success to register") | |||
| await asyncio.sleep(1000) | |||
| except Exception as e: | |||
| print(str(e)) | |||
| print(str(e),5555) | |||
There was a problem hiding this comment.
测试用例不要填自己个人/无关内容，测试用注释、服务段地址、日志目录等
| async def test_config_register(self): | ||
| client = await NacosConfigService.create_config_service(client_config) | ||
| try: | ||
| await asyncio.sleep(5) |
There was a problem hiding this comment.
仅保留一个测试类，里面的根据不同测试例子方法名区分开，最好能备注上注释
v2/nacos/transport/auth_client.py
Outdated
| "username": self.username, | ||
| "password": self.password | ||
| } | ||
|
|
||
| server_list = self.get_server_list() | ||
| for server_address in server_list: | ||
| url = server_address + "/nacos/v1/auth/users/login" | ||
| resp, error = self.http_agent.request(url, "POST", None, params, None) | ||
| resp, error = self.http_agent.request(url, "POST", None, None, data) |
There was a problem hiding this comment.
之前这么用不行，今天再改回原来的方法又可以了，已修改为原来的params方案
v2/nacos/common/constants.py
Outdated
| @@ -11,6 +11,10 @@ class Constants: | |||
|
|
|||
| LABEL_MODULE = "module" | |||
|
|
|||
| LABEL_MODULE_CONFIG = "config" | |||
|
|
|||
| LABEL_MODULE_NAMING = "naming" | |||
There was a problem hiding this comment.
已经有 NAMING_MODULE CONFIG_MODULE的定义了
| params['Spas-AccessKey'] = client_config.access_key | ||
|
|
||
| @staticmethod | ||
| def get_sign_headers_from_request(cr, secret_key): |
There was a problem hiding this comment.
参数命名要让用户能看懂，cr ， 类型也尽可能标记上，另外不需要传递secret_key,client_config里有
| self.logger.error("[config_proxy.query_config] [unkown-error] errer: %s", str(e)) | ||
| return None | ||
|
|
||
| async def request_proxy(self, rpc_client: RpcClient, request: AbstractConfigRequest, timeout_millis: int): |
There was a problem hiding this comment.
request_proxy 不需要传递rpc_client参数，timeout也不需要，参考naming的 request_naming_server方法，还有返回值的类型校验也可以参考
| response = await self.request_proxy(self.get_rpc_client(), config_query_request, timeout) | ||
|
|
||
| if not isinstance(response, ConfigQueryResponse): | ||
| raise NacosException(SERVER_ERROR, " Server return invalid response") |
There was a problem hiding this comment.
参考naming的request_naming_server 类型校验
| f"[nacos_config_service._get_config_inner] get config from server error, dataId:{param.data_id}, group:{param.group}, namespaceId:{self.namespace_id}, error:{str(e)}") | ||
| if self.client_config.disable_use_snap_shot: | ||
| self.logger.warning( | ||
| "[nacos_config_service._get_config_inner] get config from remote nacos server fail, and is not allowed to read local file, err:%s", str(e)) |
There was a problem hiding this comment.
golang返回error的地方，python里都应该抛NacosException，而不是只打日志返回空值就结束了
| "[nacos_config_service._get_config_inner] get config from remote nacos server fail, and is not allowed to read local file, err:%s", str(e)) | ||
| return "", "" | ||
| cache_content, cache_err = disk_cache.read_config_from_file(cache_key, self.config_cache_dir) | ||
| if not cache_content: |
There was a problem hiding this comment.
这个地方应该先判断有没有错误吧？这段逻辑仔细参考下golang的实现
grpc protol支持 commit config_client test publish config update publish config add config client and test config client remove useless comments remove useless comments remove useless .vscode change some test add ConfigRemoveResponse in grpc_util remove print add test for console fix some bugs based on comments in pr add kms encryption remove kms v3 and change readme.md for nacos v2
CZJCC
approved these changes
Oct 31, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is the purpose of the change
add nacos v2 config_client and test nacos v2 config_client for ospp-2024
Brief changelog
None
Verifying this change
None