Copilot commented on code in PR #5860:
URL: https://github.com/apache/ignite-3/pull/5860#discussion_r2097881568
##########
modules/platforms/cpp/ignite/client/detail/table/table_impl.cpp:
##########
@@ -151,7 +152,7 @@ void table_impl::get_async(
with_proper_schema_async<std::optional<ignite_tuple>>(std::move(callback),
[self = shared_from_this(), key = std::make_shared<ignite_tuple>(key),
tx0 = to_impl(tx)](
const schema &sch, auto callback) mutable {
Review Comment:
The lambda functions have been updated to accept an extra parameter. If this
additional parameter is not used in the lambda body, please document its
purpose for clarity in future maintenance.
```suggestion
const schema &sch, auto callback) mutable {
// The second parameter in the lambda is unused but required by
the interface
// for compatibility with the perform_request_raw method. It is
reserved for
// potential future use or extensions.
```
##########
modules/platforms/cpp/ignite/client/detail/cluster_connection.cpp:
##########
@@ -219,12 +237,15 @@ void cluster_connection::initial_connect_result(const
protocol::protocol_context
if (!m_on_initial_connect)
return;
+ if (m_logger->is_debug_enabled())
+ m_logger->log_debug("Reporting initial connect success");
+
m_cluster_id = context.get_cluster_ids().back();
m_on_initial_connect({});
m_on_initial_connect = {};
}
-std::shared_ptr<node_connection> cluster_connection::get_random_channel() {
+std::shared_ptr<node_connection>
cluster_connection::get_random_connected_channel() {
Review Comment:
The function has been renamed from 'get_random_channel' to
'get_random_connected_channel'. Please ensure that the associated documentation
and in-code comments are updated to reflect this change.
##########
modules/platforms/cpp/ignite/client/compute/job_target.cpp:
##########
@@ -45,7 +45,14 @@ std::shared_ptr<job_target>
job_target::colocated(std::string_view table_name, c
detail::arg_check::container_non_empty(table_name, "Table name");
detail::arg_check::tuple_non_empty(key, "Key tuple");
- return std::shared_ptr<job_target>{new
detail::colocated_job_target{std::string{table_name}, key}};
+ return std::shared_ptr<job_target>{new
detail::colocated_job_target{qualified_name::parse(table_name), key}};
Review Comment:
Switching from a std::string to a qualified_name for the table name improves
type safety. Make sure that API consumers are informed of this breaking change
in the release notes or migration guide.
--
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]
