You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@thrift.apache.org by "marjisound (via GitHub)" <gi...@apache.org> on 2026/10/16 09:23:29 UTC
[PR] Update underscore [thrift]
marjisound opened a new pull request, #4018:
URL: https://github.com/apache/thrift/pull/4018
Update jsdoc and underscore patch version
--
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@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update underscore [thrift]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #4018:
URL: https://github.com/apache/thrift/pull/4018#discussion_r4293132658
##########
compiler/cpp/src/thrift/generate/t_swift_generator.cc:
##########
@@ -67,6 +68,11 @@ class t_swift_generator : public t_oop_generator {
log_unexpected_ = true;
} else if( iter->first.compare("async_clients") == 0) {
async_clients_ = true;
+ } else if( iter->first.compare("async_servers") == 0) {
+ if (gen_cocoa_) {
+ throw "Async servers is not compatible with compatible with the Thrift/Cocoa library";
Review Comment:
The thrown message contains a duplicated phrase (“compatible with compatible with”) and is grammatically incorrect (“Async servers is…”). Fix the text to be clear and correct (e.g., “Async servers are not compatible with the Thrift/Cocoa library”).
##########
compiler/cpp/src/thrift/generate/t_swift_generator.cc:
##########
@@ -2517,6 +2561,141 @@ void t_swift_generator::generate_swift_service_server_implementation(ostream& ou
indent_down();
}
+ block_close(out);
+ block_close(out);
+ block_close(out);
+ out << '\n';
+ }
+
+/**
+ * Generates an async service server implementation.
+ *
+ * Implemented by generating a block for each service function that
+ * handles the processing of that function. The blocks are stored in
+ * a map and looked up via function/message name.
+ *
+ * @param tservice The service to generate an implementation for
+ */
+ // check one way behaviour
+void t_swift_generator::generate_swift_service_server_async_implementation(ostream& out,
+ t_service* tservice) {
+
+ string name = tservice->get_name() + "Processor";
+
+ indent(out) << "extension " << name << "Async : TProcessor";
+ block_open(out);
+ out << '\n';
+ indent(out) << "static let processorHandlers: ProcessorHandlerDictionary =";
+ block_open(out);
+
+ out << '\n';
+ out << indent() << "var processorHandlers = ProcessorHandlerDictionary()" << '\n' << '\n';
+
+ // generate method map for routing incoming calls
+ vector<t_function*> functions = tservice->get_functions();
+ vector<t_function*>::const_iterator f_iter;
+ for (f_iter = functions.begin(); f_iter != functions.end(); ++f_iter) {
+ t_function* tfunction = *f_iter;
+
+ string args_type = function_args_helper_struct_type(tservice, *f_iter);
+
+ out << indent() << "processorHandlers[\"" << tfunction->get_name() << "\"] = { sequenceID, inProtocol, outProtocol, handler in" << '\n'
+ << '\n';
+
+ indent_up();
+ out << indent() << "let args = try " << args_type << ".read(from: inProtocol)" << '\n'
+ << '\n'
+ << indent() << "try inProtocol.readMessageEnd()" << '\n'
+ << '\n';
+
+ if (!tfunction->is_oneway() ) { // oneway does not work properly
+
+ indent(out) << "handler." << tfunction->get_name() << "(";
Review Comment:
With `async_servers_` enabled, oneway functions are effectively ignored because the handler invocation is entirely skipped. Either generate a oneway-compatible handler call (no reply/flush, no completion/Result expected) or fail generation when a service contains oneway functions under `async_servers` so behavior isn’t silently incorrect.
##########
compiler/cpp/src/thrift/generate/t_swift_generator.cc:
##########
@@ -1795,6 +1812,33 @@ void t_swift_generator::generate_swift_service_server(ostream& out, t_service* t
out << '\n';
}
+/**
+ * Generates an async service server interface definition. In other words,
+ * the TProcess implementation for the service definition.
Review Comment:
The comment says “TProcess implementation” but the type elsewhere is `TProcessor`. Update the comment to match the actual concept/type to avoid confusion for future maintainers.
##########
.github/workflows/generate-ophan-swift-classes.yml:
##########
@@ -0,0 +1,15 @@
+on:
+ push:
+ branches:
+ - mobile-apps
+ paths:
+ - 'compiler/cpp/src/thrift/generate/t_swift_generator.cc'
+
+jobs:
+ generate-ophan-swift-classes:
+ runs-on: ubuntu-latest
+ name: Generate Ophan Swift Classes
+ steps:
+ - uses: guardian/ophan-thrift-swift@master
+ with:
+ personal-access-token: ${{ secrets.OphanThriftSwiftAccessToken }}
Review Comment:
The workflow references an action by a mutable branch (`@master`), which is vulnerable to supply-chain risks. Pin the action to an immutable commit SHA (preferred) or a version tag, and consider adding explicit `permissions:` to the workflow/job to follow least-privilege.
--
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@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update underscore [thrift]
Posted by "Jens-G (via GitHub)" <gi...@apache.org>.
Jens-G commented on PR #4018:
URL: https://github.com/apache/thrift/pull/4018#issuecomment-6220328230
:eyes:
--
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@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update underscore [thrift]
Posted by "marjisound (via GitHub)" <gi...@apache.org>.
marjisound closed pull request #4018: Update underscore
URL: https://github.com/apache/thrift/pull/4018
--
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@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update underscore [thrift]
Posted by "marjisound (via GitHub)" <gi...@apache.org>.
marjisound commented on PR #4018:
URL: https://github.com/apache/thrift/pull/4018#issuecomment-6212500232
Sorry this was an incorrect PR
--
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@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update underscore [thrift]
Posted by "marjisound (via GitHub)" <gi...@apache.org>.
marjisound closed pull request #4018: Update underscore
URL: https://github.com/apache/thrift/pull/4018
--
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@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org