Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Remove sys.exit from exception handling #1666

Closed
woop opened this issue Jun 25, 2021 · 5 comments
Closed

Remove sys.exit from exception handling #1666

woop opened this issue Jun 25, 2021 · 5 comments
Labels

Comments

@woop
Copy link
Member

woop commented Jun 25, 2021

We're currently calling sys.exit on exception handling here https://github.com/feast-dev/feast/blob/master/sdk/python/feast/feature_store.py#L301, which will cause the Feast (and wrapping) process to exit. Instead, we should either reraise or even just let the exception bubble up.

@HeardACat
Copy link
Contributor

Is the "simple" fix just to reraise everything? Or is it more nuanced than that?
Happy to put a quick PR for it.

@achals
Copy link
Member

achals commented Jul 8, 2021

@charliec443 I think it's sufficient to not catch the exception at all if all we're doing it re-raising it - but I was looking at the code and I don't see a sys.exit call anywhere in the feature_store.py. Maybe it was removed in an intermediate PR?

@HeardACat
Copy link
Contributor

Yes I think you're right. it was progressively removed in in some of the PRs, I think namely the Redshift one.

@achals
Copy link
Member

achals commented Jul 8, 2021

Alright, going to close this out in that case!

@achals achals closed this as completed Jul 8, 2021
@achals
Copy link
Member

achals commented Jul 8, 2021

Fixed by #1641

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants