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

Use from chain exceptions in k8s enviroment #4251

Merged
merged 4 commits into from
Mar 16, 2021
Merged

Conversation

abidahmadq
Copy link

@abidahmadq abidahmadq commented Mar 15, 2021

Summary

  • Added from on each raised exception in:
  • src/prefect/environments/execution/k8s.

Changes

Importance

  • Improves stack trace when debugging errors by using clearer language.

Checklist

This PR:

  • adds a change file in the changes/ directory (if appropriate)

@abidahmadq abidahmadq requested a review from jcrist as a code owner March 15, 2021 13:50
@marvin-robot
Copy link
Member

Here I am, brain the size of a planet and they ask me to welcome you to Prefect.

So, welcome to the community @Abid1998! 🎉 🎉

@zanieb
Copy link
Contributor

zanieb commented Mar 15, 2021

Thanks for taking the time to contribute :) Note that Environments are deprecated and will be eventually removed. This looks alright otherwise though.

@abidahmadq
Copy link
Author

Hi I am planning on remaining items on this list #3306 should i do it?

@abidahmadq
Copy link
Author

Should i close this PR or will it be merged?

@cicdw
Copy link
Member

cicdw commented Mar 16, 2021

Hi @Abid1998 and thanks for contributing -- we can't merge your PR until it passes our style checks; in this case, all you need to do is run black on your changes. And in general, please be patient with our reviews - it can sometimes take a while for us to get to everyone's PRs :)

@abidahmadq
Copy link
Author

@cicdw should i fix the other mentioned items in the list #3306? Haha i had asked whether to close this PR because this code was eventually gonna be removed :D

@cicdw
Copy link
Member

cicdw commented Mar 16, 2021

Hi @Abid1998 oh haha gotcha -- I think we should still merge it! Gives you the contributor credit and we aren't removing it yet, so no harm.

And if you submitted a PR that closes #3306 entirely, we'd definitely accept it

@cicdw cicdw merged commit 2969bcd into PrefectHQ:master Mar 16, 2021
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.

4 participants