acelyc111 commented on code in PR #1272:
URL:
https://github.com/apache/incubator-pegasus/pull/1272#discussion_r1038988788
##########
src/replica/replica_config.cpp:
##########
@@ -1070,6 +1071,25 @@ void replica::on_config_sync(const app_info &info,
}
}
+void replica::update_app_name(const std::string app_name)
+{
+ if (app_name == _app_info.app_name) {
+ return;
+ }
+
+ auto old_app_name = _app_info.app_name;
+ _app_info.app_name = app_name;
+
+ auto ec = store_app_info(_app_info);
+ CHECK_EQ_PREFIX_MSG(ec,
Review Comment:
```suggestion
CHECK_EQ_PREFIX_MSG(store_app_info(_app_info),
```
##########
src/client/replication_ddl_client.cpp:
##########
@@ -1738,5 +1738,27 @@ replication_ddl_client::set_max_replica_count(const
std::string &app_name,
configuration_set_max_replica_count_rpc(std::move(req),
RPC_CM_SET_MAX_REPLICA_COUNT));
}
+error_with<configuration_rename_app_response>
+replication_ddl_client::rename_app(const std::string &old_app_name, const
std::string &new_app_name)
+{
+ if (old_app_name.empty() ||
+ !std::all_of(old_app_name.cbegin(),
+ old_app_name.cend(),
+ (bool (*)(int))replication_ddl_client::valid_app_char))
+ return FMT_ERR(ERR_INVALID_PARAMETERS, "invalid old_app_name.");
Review Comment:
It seems this code is duplicated some times, could you add a macros like:
```
VALIDATE_TABLE_NAME(table_name, msg) \
if (table_name.empty() || ...) { \
return FMT_ERR(ERR_INVALID_PARAMETERS, #msg);
```
you can do it in another patch and refactor some old code.
##########
src/replica/replica.h:
##########
@@ -496,6 +496,7 @@ class replica : public serverlet<replica>, public
ref_counter, public replica_ba
clear_on_failure(replica_stub *stub, replica *rep, const std::string
&path, const gpid &pid);
void update_app_max_replica_count(int32_t max_replica_count);
+ void update_app_name(const std::string app_name);
Review Comment:
```suggestion
void update_app_name(const std::string &app_name);
```
##########
src/replica/replica_config.cpp:
##########
@@ -1070,6 +1071,25 @@ void replica::on_config_sync(const app_info &info,
}
}
+void replica::update_app_name(const std::string app_name)
Review Comment:
```suggestion
void replica::update_app_name(const std::string &app_name)
```
##########
src/client/replication_ddl_client.cpp:
##########
@@ -1738,5 +1738,27 @@ replication_ddl_client::set_max_replica_count(const
std::string &app_name,
configuration_set_max_replica_count_rpc(std::move(req),
RPC_CM_SET_MAX_REPLICA_COUNT));
}
+error_with<configuration_rename_app_response>
+replication_ddl_client::rename_app(const std::string &old_app_name, const
std::string &new_app_name)
+{
+ if (old_app_name.empty() ||
+ !std::all_of(old_app_name.cbegin(),
+ old_app_name.cend(),
+ (bool (*)(int))replication_ddl_client::valid_app_char))
+ return FMT_ERR(ERR_INVALID_PARAMETERS, "invalid old_app_name.");
+ if (!std::all_of(new_app_name.cbegin(),
+ new_app_name.cend(),
+ (bool (*)(int))replication_ddl_client::valid_app_char)) {
+ return FMT_ERR(ERR_INVALID_PARAMETERS,
+ "Not support format for new_app_name. You can use "
+ "letters and numbers, and also char in ['.', ':', "
+ "'_'].");
Review Comment:
How about simplified by "Only 0-9a-zA-Z.:_ are valid" ? You can append it to
the message in the macro mentioned above.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
