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

feat: Add task handler api c++ #334

Merged
merged 14 commits into from
Jul 12, 2023
Merged

feat: Add task handler api c++ #334

merged 14 commits into from
Jul 12, 2023

Conversation

ddiakiteaneo
Copy link
Contributor

No description provided.

@ddiakiteaneo ddiakiteaneo self-assigned this Jul 4, 2023
@github-actions
Copy link

github-actions bot commented Jul 4, 2023

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
515 466 90% 0% 🟢

New Files

No new covered files...

Modified Files

No covered modified files...

updated for commit: 7d892ba by action🐍

if (init_request.payload().data_complete()) {
std::string temp = init_request.payload().data();
payload.resize(temp.length());
for (size_t i = 0; i < temp.length(); i++) {
payload[i] = static_cast<std::byte>(temp[i]);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is correct, you cannot have data_complete AND data

Comment on lines 85 to 88
chuncks.push_back(datachunck.data());
}

size_t size = chuncks.size();

auto payload_data = new std::byte[size];
int address = 0;
for (auto iter = chuncks.begin(); iter != chuncks.end(); iter++) {
std::string temp_str = *iter;

for (size_t i = 0; i < temp_str.length(); i++) {
payload_data[i + address] = static_cast<std::byte>(temp_str[i]);
}
address += temp_str.length();
}

payload.resize(size);
std::copy_n(payload_data, size, std::back_inserter(payload));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

1 : what's the point of payload_data ? You already have payload
2 : Both payload_data and payload are local, they cannot be accessed !

Copy link
Contributor

@dbrasseur-aneo dbrasseur-aneo left a comment

Choose a reason for hiding this comment

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

Only details, looks good as is

Comment on lines +67 to +69
static auto to_request_stream(const std::vector<armonik::api::grpc::v1::TaskRequest> &task_requests,
armonik::api::grpc::v1::TaskOptions task_options, size_t chunk_max_size)
-> std::vector<std::future<std::vector<armonik::api::grpc::v1::agent::CreateTaskRequest>>>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
static auto to_request_stream(const std::vector<armonik::api::grpc::v1::TaskRequest> &task_requests,
armonik::api::grpc::v1::TaskOptions task_options, size_t chunk_max_size)
-> std::vector<std::future<std::vector<armonik::api::grpc::v1::agent::CreateTaskRequest>>>;
static std::vector<std::future<std::vector<armonik::api::grpc::v1::agent::CreateTaskRequest>>> to_request_stream(const std::vector<armonik::api::grpc::v1::TaskRequest> &task_requests,
armonik::api::grpc::v1::TaskOptions task_options, size_t chunk_max_size);

Comment on lines +237 to +241
for (auto task_request = task_requests.begin(); task_request != task_requests.end(); ++task_request) {
const bool is_last = task_request == task_requests.end() - 1 ? true : false;

async_chunk_payload_tasks.push_back(task_chunk_stream(*task_request, is_last, chunk_max_size));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
for (auto task_request = task_requests.begin(); task_request != task_requests.end(); ++task_request) {
const bool is_last = task_request == task_requests.end() - 1 ? true : false;
async_chunk_payload_tasks.push_back(task_chunk_stream(*task_request, is_last, chunk_max_size));
}
for (auto&& task_request : task_requests) {
const bool is_last = task_request == task_requests.end() - 1 ? true : false;
async_chunk_payload_tasks.push_back(task_chunk_stream(*task_request, is_last, chunk_max_size));
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure between auto& and auto&&

@ddiakiteaneo ddiakiteaneo merged commit 213ed8c into main Jul 12, 2023
@ddiakiteaneo ddiakiteaneo deleted the ddi/task-handler branch July 12, 2023 14:53
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.

3 participants