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

DWS | Task 1 | Ершов Вадим #19

Merged
merged 13 commits into from
Mar 2, 2024
Merged

Conversation

vadim01er
Copy link
Contributor

No description provided.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 10332 lines exceeds the maximum allowed for the inline comments feature.

import static ru.vk.itmo.test.vadimershov.utils.MemorySegmentUtil.toEntity;
import static ru.vk.itmo.test.vadimershov.utils.MemorySegmentUtil.toMemorySegment;

public class DaoHttpServer extends HttpServer {
Copy link
Contributor

Choose a reason for hiding this comment

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

Не хватает тут обработок неожиданных ошибок, если вдруг упадет ошибка при обработке, то что мы показывали на предыдущем занятии

Request.METHOD_DELETE
);

private final ReferenceDao dao;
Copy link
Contributor

Choose a reason for hiding this comment

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

В полях лучше использовать как можно более абстрактный класс

return new Response(Response.NOT_FOUND, Response.EMPTY);
}

byte[] value = toByteArray(entry.value());
Copy link
Contributor

Choose a reason for hiding this comment

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

тут лишнее копирование, которого можно было бы избежать, но пришлось бы попотеть, чтобы сразу отдавать в ответ из мемори сегмента

Copy link
Contributor Author

@vadim01er vadim01er Feb 25, 2024

Choose a reason for hiding this comment

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

я посмотрел как реализован write Response до сокета, как я понимаю можно переопределить так чтоб можно было передать сразу byteBuffer, но тут тоже будет аллокация, или я не в ту сторону думаю?

@atimofeyev atimofeyev self-requested a review February 25, 2024 09:21
}

@Override
public CompletableFuture<Void> start() throws IOException {
Copy link
Contributor

@atimofeyev atimofeyev Feb 25, 2024

Choose a reason for hiding this comment

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

старт и стоп лучше сделать синхронизированными, и сделать защиту всякую, а то если несколько раз вызывать - будут всякие неопределенности

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 10367 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 10358 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 10358 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 10362 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 10384 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 10384 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 10384 lines exceeds the maximum allowed for the inline comments feature.

@atimofeyev atimofeyev merged commit db97ff7 into polis-vk:main Mar 2, 2024
2 checks passed
osokindm pushed a commit to osokindm/2024-highload-dht that referenced this pull request Mar 6, 2024
* Task 1 | Implement DaoHttpServer

* Task 1 | Fix codeclimate

* Task 1 | Fix codeclimate 2

* Task 1 | Add report.md

* Task 1 | Fix comments

* Task 1 | Fix codeclimate

* Task 1 | Fix codeclimate

* Task 1 | Fix exception handler

* Task 1 | Fix exception handler

* Task 1 | Fix exception handler

---------

Co-authored-by: ruarsv5 <[email protected]>
Co-authored-by: atimofeyev <[email protected]>
axothy pushed a commit to axothy/2024-highload-dht that referenced this pull request Mar 13, 2024
* Task 1 | Implement DaoHttpServer

* Task 1 | Fix codeclimate

* Task 1 | Fix codeclimate 2

* Task 1 | Add report.md

* Task 1 | Fix comments

* Task 1 | Fix codeclimate

* Task 1 | Fix codeclimate

* Task 1 | Fix exception handler

* Task 1 | Fix exception handler

* Task 1 | Fix exception handler

---------

Co-authored-by: ruarsv5 <[email protected]>
Co-authored-by: atimofeyev <[email protected]>
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