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

Remove all usage of exc_str() #6142

Merged
merged 1 commit into from
Nov 5, 2021
Merged

Remove all usage of exc_str() #6142

merged 1 commit into from
Nov 5, 2021

Conversation

mih
Copy link
Member

@mih mih commented Nov 4, 2021

@mih mih linked an issue Nov 4, 2021 that may be closed by this pull request
@mih mih added the semver-internal Changes only affect the internal API label Nov 4, 2021
@mih mih force-pushed the excstr branch 2 times, most recently from 91f16b8 to 3567127 Compare November 5, 2021 09:46
@codecov
Copy link

codecov bot commented Nov 5, 2021

Codecov Report

Merging #6142 (ff81b11) into maint (dccedaf) will increase coverage by 0.37%.
The diff coverage is 69.93%.

❗ Current head ff81b11 differs from pull request most recent head 6550e56. Consider uploading reports for the commit 6550e56 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##            maint    #6142      +/-   ##
==========================================
+ Coverage   85.97%   86.35%   +0.37%     
==========================================
  Files         312      312              
  Lines       42215    42249      +34     
==========================================
+ Hits        36296    36484     +188     
+ Misses       5919     5765     -154     
Impacted Files Coverage Δ
datalad/distribution/update.py 98.15% <ø> (-0.01%) ⬇️
datalad/downloaders/s3.py 57.36% <0.00%> (-0.59%) ⬇️
datalad/downloaders/tests/test_s3.py 51.63% <0.00%> (+4.88%) ⬆️
datalad/interface/download_url.py 98.11% <ø> (-0.02%) ⬇️
datalad/interface/rerun.py 95.83% <ø> (-0.02%) ⬇️
datalad/interface/results.py 96.92% <ø> (-0.03%) ⬇️
datalad/metadata/extractors/image.py 19.35% <0.00%> (ø)
datalad/metadata/extractors/tests/test_image.py 24.00% <ø> (-2.93%) ⬇️
datalad/support/github_.py 73.07% <0.00%> (-0.18%) ⬇️
datalad/support/sshconnector.py 82.79% <ø> (-0.07%) ⬇️
... and 97 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 80d93a5...6550e56. Read the comment docs.

@yarikoptic
Copy link
Member

FWIW: I still can't fathom the value of adding a single use ce variable (longer code etc) and not just aliasing exc_str to be CapturedException besides rare cases of debugging whenever the exception manages to bubble up through all the generators and results handling in --dbg mode.

@mih
Copy link
Member Author

mih commented Nov 5, 2021

FWIW: I still can't fathom the value of adding a single use ce variable (longer code etc) and not just aliasing exc_str to be CapturedException besides rare cases of debugging whenever the exception manages to bubble up through all the generators and results handling in --dbg mode.

I was expecting this comment (again). I will respond in the same fashion as before. I just implemented the concept that was discussed and put in writing. With this PR the concept is fully implemented. We can now start the discussion what the next concept will be, and then we can implement that too. ;-)

@mih mih merged commit 5bbe9d1 into datalad:maint Nov 5, 2021
@mih mih deleted the excstr branch November 5, 2021 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-internal Changes only affect the internal API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Discontinue use of exc_str() throughout the code base
2 participants