[launch_manager] use score::Result instead of score::cpp::expected - #350
Open
Narasimha2211 wants to merge 1 commit into
Open
[launch_manager] use score::Result instead of score::cpp::expected#350Narasimha2211 wants to merge 1 commit into
Narasimha2211 wants to merge 1 commit into
Conversation
…concurrency queues MpscBoundedQueue and MPMCConcurrentQueue (introduced in eclipse-score#333) used score::cpp::expected/expected_blank<ConcurrencyErrc>, whereas the rest of launch_manager's public error-returning APIs use score::Result. Per the PR eclipse-score#333 review discussion, switch these two queues over to score::Result for consistency. - ConcurrencyErrc's underlying type changes from uint8_t to score::result::ErrorCode (int32_t), as required to construct a score::result::Error from it. - Add ConcurrencyErrorDomain and an ADL-visible MakeError() overload for ConcurrencyErrc, following the same pattern as ExecErrorDomain in score/launch_manager/src/execution_error.h. - push()/wait()/stop() now return score::Result<void>; pop() returns score::Result<T>. Callers that only relied on has_value()/error()/ operator bool() (workerthread.hpp, graph.cpp, process_info_node.cpp) did not need changes since they use `auto` for the result type. - Update BUILD deps: add @score_baselibs//score/result to both queue targets, and drop the now-unused @score_baselibs//score/language/futurecpp dependency from mpmc_concurrent_queue (mpsc_bounded_queue keeps it for score/assert.hpp). Fixes eclipse-score#338 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: asymtote <152078151+asymtote@users.noreply.github.com>
Narasimha2211
temporarily deployed
to
workflow-approval
July 23, 2026 05:31 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
Narasimha2211
temporarily deployed
to
workflow-approval
July 23, 2026 05:31 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
License Check Results🚀 The license check job ran with the Bazel command: bazel run --lockfile_mode=error //:license-checkStatus: Click to expand output |
|
The created documentation from the pull request is available at: docu-html |
Contributor
|
The eclipsefdn/eca check is failing. @Narasimha2211 please make sure that you have an eclipse account that is linked to your github account. We will only be able to merge your changes, once this check is successful. |
Contributor
@Narasimha2211 you find the instructions here https://eclipse-score.github.io/score/main/contribute/general/contribution_attribution.html# |
This file contains hidden or 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
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.
Summary
Closes #338.
This issue is a "spin off from review comment" auto-generated from a review exchange on #333, about the newly-added
MpscBoundedQueue/MPMCConcurrentQueueclasses usingscore::cpp::expected_blank<ConcurrencyErrc>.Scope
Scoped this to exactly the concurrency queue subsystem rather than every
score::cpp::expectedusage underlaunch_manager/, which also includes an unrelatedconfiguration/flatbuffer_*subsystem using its ownIConfigLoader::Errorenum across ~20 functions, and a test file mocking a third-partyscore::osAPI signature that must not change. Widening the scope to those would have been a much larger, speculative change not discussed in this issue.Files changed (5)
score/launch_manager/src/daemon/src/common/concurrency/concurrency_error_domain.hpp-- newConcurrencyErrorDomain(score::result::ErrorDomainsubclass) and an ADL-visibleMakeError(ConcurrencyErrc, ...), mirroring the existingExecErrorDomainpattern inexecution_error.h. Also changedConcurrencyErrc's underlying type fromuint8_ttoscore::result::ErrorCode(int32_t) -- required byscore::result::Error's constructor constraint (IsValidErrorCodeEnum).mpsc_bounded_queue.hpp,mpmc_concurrent_queue.hpp-- allscore::cpp::expected_blank<ConcurrencyErrc>/score::cpp::expected<T, ConcurrencyErrc>return types converted toscore::Result<void>/score::Result<T>;score::cpp::make_unexpected(...)->score::MakeUnexpected(...);score::cpp::blank{}->{}.mpmc_concurrent_queue_test.cpp-- updated 5 local variable declarations to match.BUILD-- added@score_baselibs//score/resultdep to both queue targets; dropped the now-fully-unused@score_baselibs//score/language/futurecppdep frommpmc_concurrent_queue(kept it formpsc_bounded_queue, which still needsscore/assert.hppfrom that package).mpsc_bounded_queue_test.cpp,workerthread.hpp,mpmc_concurrent_queue_benchmark.cpp, and the two external callers (graph.cpp,process_info_node.cpp) needed no changes -- they consume results viaauto/.has_value()/.error()/operator bool(), all API-identical betweenscore::cpp::expectedandscore::Result.How I tested this
No Bazel binary was available in the environment I worked in, so a build wasn't feasible. Verified by evidence-based code review instead:
score::Result/score::result::Error/ErrorDomainimplementations fromeclipse-score/baselibsto confirm API compatibility (has_value(),error(),operator bool(), implicit construction fromunexpected<E>, and theLogStream& operator<<(LogStream&, const Error&)overload that keeps existingLM_LOG_ERROR() << ... << foo.error()call sites compiling unchanged).ExecErrorDomain/ExecErrcpattern already used elsewhere in this codebase and replicated it exactly forConcurrencyErrc.score::cpp::expected/expected_blank/score/expected.hppreferences in the touched scope -- none remain.Given I couldn't build, a CI run / maintainer build check would be very welcome on this one.
Commits are DCO-signed (
git commit -s).