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.

Sign up for GitHub

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

Jump to bottom

75 gracefully shutdown server #101

Merged
merged 25 commits into from
Jul 11, 2024
Merged

75 gracefully shutdown server #101

merged 25 commits into from
Jul 11, 2024

Conversation

Christiantyemele
Copy link
Collaborator

No description provided.

@Christiantyemele Christiantyemele linked an issue Jun 29, 2024 that may be closed by this pull request
Gracefully shutdown of server #75
Closed
francis-pouatcha
francis-pouatcha previously requested changes Jun 30, 2024
View reviewed changes
Copy link
Member

@francis-pouatcha francis-pouatcha left a comment

Choose a reason for hiding this comment

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

is there a way to provide some tests?

@Christiantyemele
Copy link
Collaborator Author

is there a way to provide some tests?

Ok i'll provide some tests

IngridPuppet
IngridPuppet reviewed Jul 2, 2024
View reviewed changes
Copy link
Collaborator

@IngridPuppet IngridPuppet left a comment

Choose a reason for hiding this comment

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

The root intent of this graceful shutdown ticket was to unmount all plugins. It is something you must primarily handle.

I also notice that there are many changes due to formatting. To avoid that, we relied on the autoformat of vscode. Can you double check that you don't have any customization that ruins many files (exclude did-utils)?

IngridPuppet
IngridPuppet requested changes Jul 9, 2024
View reviewed changes
mediator-server/Cargo.toml Outdated Show resolved Hide resolved
generic-server/Cargo.toml Outdated Show resolved Hide resolved
generic-server/src/lib.rs Outdated Show resolved Hide resolved
generic-server/src/lib.rs Outdated Show resolved Hide resolved
generic-server/src/main.rs Outdated Show resolved Hide resolved
generic-server/src/plugin/container.rs Outdated Show resolved Hide resolved
generic-server/src/plugin/container.rs Outdated Show resolved Hide resolved
generic-server/src/plugin/container.rs Outdated Show resolved Hide resolved
generic-server/src/plugin/container.rs Outdated Show resolved Hide resolved
generic-server/src/plugin/container.rs Outdated Show resolved Hide resolved
IngridPuppet
IngridPuppet requested changes Jul 11, 2024
View reviewed changes
generic-server/Cargo.toml Outdated Show resolved Hide resolved
generic-server/src/main.rs Outdated Show resolved Hide resolved
generic-server/src/main.rs Outdated Show resolved Hide resolved
generic-server/src/plugin/container.rs Outdated Show resolved Hide resolved
generic-server/src/plugin/container.rs Outdated Show resolved Hide resolved
generic-server/src/lib.rs Outdated Show resolved Hide resolved
generic-server/src/lib.rs Show resolved Hide resolved
generic-server/src/main.rs Outdated Show resolved Hide resolved
generic-server/src/main.rs Show resolved Hide resolved
generic-server/src/plugin/container.rs Show resolved Hide resolved
@Christiantyemele Christiantyemele requested review from francis-pouatcha and removed request for francis-pouatcha and Hermann-Core July 11, 2024 14:52
Christiantyemele
Christiantyemele commented Jul 11, 2024
View reviewed changes
Copy link
Collaborator Author

@Christiantyemele Christiantyemele left a comment

Choose a reason for hiding this comment

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

Resolved

@Christiantyemele
Copy link
Collaborator Author

is there a way to provide some tests?

Since testing the shuttdown of the server itself is an issue, I opted for testing the unloading of the plugins on shutdown which can simulate the gracefull shutdown.

@Christiantyemele Christiantyemele dismissed francis-pouatcha’s stale review July 11, 2024 14:57

Changes already addressed .

@Christiantyemele Christiantyemele merged commit c1c919e into main Jul 11, 2024
2 checks passed
@IngridPuppet IngridPuppet deleted the 75-gracefully-shutdown-server branch July 11, 2024 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@IngridPuppet IngridPuppet IngridPuppet approved these changes

@francis-pouatcha francis-pouatcha francis-pouatcha left review comments

Assignees

@Christiantyemele Christiantyemele

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Gracefully shutdown of server
3 participants
@Christiantyemele @francis-pouatcha @IngridPuppet