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!

  1. Leave a comment below claiming a sub-module
  2. 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"
    
  3. Fix all of these cases.
    • see #3320 for an example
  4. 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

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

Most upvoted comments

I’ll take the next one on the list! (and more if others don’t volunteer after Hacktoberfest)

  • src/prefect/contrib/tasks/databricks

@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 and src/prefect/engine. Thanks

i 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!