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

Provide ConfigureOriginAPI a cancel context that can be cancelled on shutdown #431

Merged
merged 3 commits into from
Dec 5, 2023
Merged

Provide ConfigureOriginAPI a cancel context that can be cancelled on shutdown #431

merged 3 commits into from
Dec 5, 2023

Conversation

haoming29
Copy link
Contributor

Closes #350

@haoming29 haoming29 added the internal Internal code improvements, not user-facing label Nov 30, 2023
@haoming29 haoming29 added this to the v7.3.0 milestone Nov 30, 2023
@haoming29 haoming29 requested a review from turetske December 4, 2023 19:40
turetske
turetske requested changes Dec 5, 2023
View reviewed changes
Copy link
Collaborator

@turetske turetske left a comment

Choose a reason for hiding this comment

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

Can you deal with the merge conflicts and mirror this on the cmd_serve?

@haoming29
Copy link
Contributor Author

I solved the merge conflict, but I couldn't find where cmd_serve lives. @turetske Could you point me to it? Thanks

@haoming29 haoming29 requested a review from turetske December 5, 2023 20:21
turetske
turetske requested changes Dec 5, 2023
View reviewed changes
Copy link
Collaborator

@turetske turetske left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@haoming29 It should be in cmd/cache_serve.go if you've rebased off of main since yesterday. And I just realized I made a spelling mistake in my previous review, I'm sorry about that.

@haoming29
Copy link
Contributor Author

@haoming29 It should be in cmd/cache_serve.go if you've rebased off of main since yesterday. And I just realized I made a spelling mistake in my previous review, I'm sorry about that.

No worries. Will add it

@haoming29 haoming29 requested a review from turetske December 5, 2023 21:09
turetske
turetske approved these changes Dec 5, 2023
View reviewed changes
Copy link
Collaborator

@turetske turetske left a comment

Choose a reason for hiding this comment

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

LGTM!

@turetske turetske merged commit 138180b into PelicanPlatform:main Dec 5, 2023
6 checks passed
@haoming29 haoming29 deleted the imprv-test-shutdown branch December 6, 2023 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@turetske turetske turetske approved these changes

Assignees
No one assigned
Labels
internal Internal code improvements, not user-facing
Projects
None yet
Milestone
v7.3.0
Development

Successfully merging this pull request may close these issues.

Provide ConfigureOriginAPI a context object that can be cancelled on shutdown
2 participants
@haoming29 @turetske