refactor(orchestrator): add ResultToGetTargetGraphResponse and migrate call sites#179
refactor(orchestrator): add ResultToGetTargetGraphResponse and migrate call sites#179justinwon777 wants to merge 1 commit into
Conversation
…e call sites Adds orchestrator.ResultToGetTargetGraphResponse (and its private chunkTargets helper), copied from core/common, and switches native_orchestrator.go and example/cmd/query-bench over to it. ResultToGetTargetGraphResponse's only real consumer is orchestrator's GetTargetGraph; keeping it there instead of a shared package avoids a grab-bag dependency for something with a single production caller. core/common's copy is left in place for now; a follow-up PR removes the duplicated code once all consumers have migrated. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
| const _cancelCheckInterval = 4096 | ||
|
|
||
| // ResultToGetTargetGraphResponse converts a Result to a GetTargetGraphResponse | ||
| func ResultToGetTargetGraphResponse(ctx context.Context, result targethasher.Result) ([]*tangopb.GetTargetGraphResponse, error) { |
There was a problem hiding this comment.
Why is this part of orchestrator package? Given that it is trying to map the tango result to the proto response, it should be in own mapper package (not orchestrator) which is added as a dependency in the handler code (In this case controller since controller is acting as both handler + controller)
| const _cancelCheckInterval = 4096 | ||
|
|
||
| // ResultToGetTargetGraphResponse converts a Result to a GetTargetGraphResponse | ||
| func ResultToGetTargetGraphResponse(ctx context.Context, result targethasher.Result) ([]*tangopb.GetTargetGraphResponse, error) { |
There was a problem hiding this comment.
we're leaking the proto definition to orchestrator now?
Addressing both @liujsj @sywhang: The proto response is not Tango's API response, but it stays in |
3 participants
Adds orchestrator.ResultToGetTargetGraphResponse, copied from core/common. This is only called by orchestrator and the benchmark tool, so I migrated them here also. A follow-up PR will remove the duplicated code once all of core/common is migrated.
Test Plan
unit test
Issue