prefect: Use 'from' to explicitly chain exceptions
Current behavior
I wanted to spend a little time contributing this weekend, so I ran pylint
over this repo. I see this project has a .pylintrc
but it looks like pylint
isn’t run in CI, so some things do show up.
I found one class of warning from pylint
that I think is worth addressing: explicitly re-raising exceptions in try-catches with from
.
The newest version of pylint
added a check for this. You can see details in “Explicit Exception Chaining”. That PEP describes a way to improve stack traces from try-catched exceptions.
without from
try:
{}["a"]
except KeyError:
raise RuntimeError("hey this does not exist")
Traceback (most recent call last):
File "<stdin>", line 2, in <module>
KeyError: 'a'
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "<stdin>", line 4, in <module>
RuntimeError: hey this does not exist
with from
try:
{}["a"]
except KeyError as err:
raise RuntimeError("hey this does not exist") from err
Traceback (most recent call last):
File "<stdin>", line 2, in <module>
KeyError: 'a'
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "<stdin>", line 4, in <module>
RuntimeError: hey this does not exist
I think this language of “was the direct cause of the following exception” is clearer, and helps you to understand exactly where things started going wrong.
Proposed behavior
Would you be open to a pull request that changes try-catched exceptions in prefect
to use this pattern? I wanted to ask first because right now there are 1334 places where that happens so it would touch a lot of code.
you can see them all like this:
pylint src/prefect/ | grep raise-missing-from > pylint.txt
I think this change would improve prefect
and help save users some debugging time. Especially users who are not very experienced in Python.
Thanks for your time and consideration.
Notes for Hacktoberfest Contributors
This issue has been left open for Hacktoberfest 2020 contributors. If that describes you, welcome!
- Leave a comment below claiming a sub-module
- Identify all cases of this issue in that submodule by running code like the following in a terminal:
pylint src/prefect/engine/cloud | grep "raise-missing-from"
- Fix all of these cases.
- see #3320 for an example
- Open a pull request, following the steps in https://github.com/PrefectHQ/prefect/pulls
- if there is already a
changes/issue3306.yaml
, add your name to the list of contributors - if there is not already a
changes/issue3306.yaml
, create one in your pull request, like in #3320
- if there is already a
sub-modules with this problem
- ~src/prefect/client~ - @jameslamb (#3320)
- ~src/prefect/core~ - @brainless (#3383)
- ~src/prefect/engine~ - @brainless (#3383)
- src/prefect/environments/execution/dask
- src/prefect/environments/execution/k8s
- ~src/prefect/environments/storage~ - @shalika10
- src/prefect/tasks/airtable
- ~src/prefect/tasks/aws~ - @heyitskevin
- src/prefect/tasks/azure
- src/prefect/tasks/azureml
- ~src/prefect/tasks/databricks~ - @rikturr (#3448)
- src/prefect/tasks/dbt
- src/prefect/tasks/dropbox
- ~src/prefect/tasks/gcp~ - @ericlundy87 (#3326)
- src/prefect/tasks/great_expectations
- src/prefect/tasks/gsheets
- src/prefect/tasks/kubernetes
- ~src/prefect/tasks/mysql~ - @luthrap (#3428)
- ~src/prefect/tasks/postgres~ - @brunocasarotti
- src/prefect/tasks/redis
- src/prefect/tasks/rss
- src/prefect/tasks/snowflake
- ~src/prefect/tasks/spacy~ - @sp1thas
- src/prefect/tasks/template
- src/prefect/tasks/twitter
- ~src/prefect/utilities~ - @gabrielcalderon (#3429)
NOTE: This issue is not urgent. It’s ok to claim a sub-module now and not contribute until Hacktoberfest begins on October 1st.
About this issue
- Original URL
- State: closed
- Created 4 years ago
- Reactions: 5
- Comments: 43
Commits related to this issue
- luthrap|#3306|Fix mysql module for explicit chaining — committed to luthrap/prefect by luthrap 4 years ago
- Merge pull request #3428 from luthrap/3306_mysql luthrap|#3306|Fix mysql module for explicit chaining — committed to PrefectHQ/prefect by joshmeek 4 years ago
- shalika10|PrefectHQ#3306|Fix environments -storage module for explicit chaining — committed to shalika10/prefect by deleted user 4 years ago
- Merge pull request #3458 from shalika10/3306_storage_exception_fix shalika10|PrefectHQ#3306|Fix environments -storage module for explic… — committed to PrefectHQ/prefect by joshmeek 4 years ago
I’ll take the next one on the list! (and more if others don’t volunteer after Hacktoberfest)
@jameslamb @joshmeek yayy @jcrist my first PR got merged 😃
Can I take
src/prefect/tasks/aws
?Hi, I could take up spacy tasks.
Hi guys, could “src/prefect/tasks/postgres” be asigned to me? It would be my first contribution too.
hey this is my first contribution !! please assign me src/prefect/environments/storage
Done for mysql. Can pick for other sub-modules if there are no other volunteers after Hacktoberfest.
Hello @jameslamb and team, thanks for sharing this super well documented issue. I want to take up
src/prefect/core
andsrc/prefect/engine
. Thanksi can take
src/prefect/contrib/tasks/mysql
I can take
src/prefect/utilities
👍🏻I also can take several others if there’s no other volunteers after Hacktoberfest.
hey can I take
src/prefect/tasks/gcp
?I just submitted a PR for
src/prefect/tasks/twitter
since today is the last day for Hacktoberfest. I can take several of the ones left is no one else is doing them.You should do a bare
raise
, we want the original error to be raised.A few days ago, some changes were made to the Hacktoberfest accounting rules to try to combat spam PRs (https://github.com/digitalocean/hacktoberfest/pull/596). I don’t think those changes are widely known yet.
I’ll leave it to Prefect maintainers whether or not they want to add the
hacktoberfest
label here to participate.Yeah I’m in favor of holding off and tackling in smaller pieces once October starts next Thursday 👻
@brainless Also, they don’t get counted until October 1st.
Awesome sounds good 👍
That sounds great @jameslamb!