Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Agent/Worker comm using files in Python and Cpp #421

Merged
merged 4 commits into from
Sep 28, 2023

Conversation

dbrasseur-aneo
Copy link
Contributor

No description provided.

@github-actions
Copy link

github-actions bot commented Sep 27, 2023

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
709 643 91% 0% 🟢

New Files

No new covered files...

Modified Files

No covered modified files...

updated for commit: 6c662c2 by action🐍

@dbrasseur-aneo dbrasseur-aneo changed the title fix: Agent/Worker comm using files fix: Agent/Worker comm using files in Python and Cpp Sep 27, 2023
@dbrasseur-aneo dbrasseur-aneo marked this pull request as ready for review September 27, 2023 08:27
@dbrasseur-aneo dbrasseur-aneo self-assigned this Sep 27, 2023
@dbrasseur-aneo
Copy link
Contributor Author

Cla bot not giving the comment...

@@ -89,7 +84,8 @@ class TaskHandler {
* @param data The result data
* @return A future containing a vector of ResultReply
*/
std::future<armonik::api::grpc::v1::agent::ResultReply> send_result(std::string key, absl::string_view data);
std::future<armonik::api::grpc::v1::agent::NotifyResultDataResponse> send_result(std::string key,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is unfortunate that it changes the API. I think you should wrap this message somehow

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd return nothing and throw if there is an issue

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does this change in this PR ?

@aneojgurhem aneojgurhem merged commit dafff72 into main Sep 28, 2023
30 checks passed
@aneojgurhem aneojgurhem deleted the db/dataFromFilesCpp branch September 28, 2023 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants