search

clj-python / libpython-clj

Python bindings for Clojure
Eclipse Public License 2.0
1.05k stars 68 forks source link

ISSUE-159: possible fix for information hiding #177

Closed jjtolton closed 2 years ago

jjtolton commented 2 years ago

Closes: https://github.com/clj-python/libpython-clj/issues/159

@cnuernber I'm not sure about the background on that try/catch, so please double check this for me. Removing it exposes the error message @behrica is seeking.

jjtolton commented 2 years ago

Ah I see the case now. builtins.list. Good thing you setup these checks!

cnuernber commented 2 years ago

Is this PR ready for review/testing?

jjtolton commented 2 years ago

Provided you think it makes logical sense, I’ll move it out of draft

On Wed, Oct 20, 2021 at 8:54 AM Chris Nuernberger @.***> wrote:

Is this PR ready for review/testing?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/clj-python/libpython-clj/pull/177#issuecomment-947636836, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACPJX4644ONW76D76MZL6S3UH23YLANCNFSM5F3VACNQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

cnuernber commented 2 years ago

Oh definitely it makes logical sense. It is hard to get the metadata generation to work correctly and not hide errors as often times simply reading python attributes causes errors but we should not hide errors due to importing modules. This is a great PR as that has bitten me and I am sure many other people.

jjtolton commented 2 years ago

Great. I’ll move it out of draft shortly

On Wed, Oct 20, 2021 at 9:22 AM Chris Nuernberger @.***> wrote:

Oh definitely it makes logical sense. It is hard to get the metadata generation to work correctly and not hide errors as often times simply reading python attributes causes errors but we should not hide errors due to importing modules. This is a great PR as that has bitten me and I am sure many other people.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/clj-python/libpython-clj/pull/177#issuecomment-947662125, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACPJX4YST5IN5ZUFBHWIZILUH27A7ANCNFSM5F3VACNQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

cnuernber commented 2 years ago

So this will expose error information if the module the user tries to load fails at import but not of some nested module does, is that correct?

jjtolton commented 2 years ago

So this will expose error information if the module the user tries to load fails at import but not of some nested module does, is that correct?

I narrowly checked to make sure that it covered the complaint in https://github.com/clj-python/libpython-clj/issues/159 only. It would be possible to extract the StackTrace, they can be converted to first class objects https://docs.python.org/3/library/traceback.html#traceback.extract_tb. There would need to be some magic done to convert the pointer.