-
Notifications
You must be signed in to change notification settings - Fork 42
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
Allow downstream projects to import func-e #446
Conversation
CI is failing due to an unrelated test failing func-e/lint/last_known_envoy_test.go Line 37 in 9c44e50
|
// Wait for run to exit or an explicit stop. | ||
select { | ||
case <-ctx.Done(): | ||
return nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't we kill the envoy process in this case? (not sure if there's such function in func-e now)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't this done in the called func libs
Line 78 in 82577c9
// At this point, shutdown hooks have run and Envoy is interrupted. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok cool but it seems to me that the code block only reaches when the OS signal was sent. Is there any way to verify that it works as intended? e.g. passing the fake config that runs indefinitely in tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you point me to some tests in the code base that verify this, that I can peruse here ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry no idea. Could you do that by yourself ?
* Define an api that allows go projects to use func-e as a library Takes #434 forward Fixes: #433 Signed-off-by: Arko Dasgupta <[email protected]>
Signed-off-by: Arko Dasgupta <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
YOLO
Takes #434 forward
Fixes: #433