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 custom item retrieval in the params object #1923

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pevogam
Copy link
Contributor

@pevogam pevogam commented Jan 17, 2019

Not only does this hide a proper stack trace of where the missing
parameter was detected but it also makes it impossible to use
other useful methods like pop() with a default and so on, all
because of the unnecessary custom ParamNotFound error which is
never caught in an exception clause at the abc collection on
a higher level.

Signed-off-by: Plamen Dimitrov [email protected]

@pevogam
Copy link
Contributor Author

pevogam commented Jan 17, 2019

Some more comments:

  • This pull request is a suggestion that I am open about discussing. There might be better approaches than the one I had to do in order to make my own debugging easier and restore the functionality that was previously available.
  • AFAIK such problems can be reproduced on python3 and not python2 due to changes in the collections modules.

Not only does this hide a proper stack trace of where the missing
parameter was detected but it also makes it impossible to use
other useful methods like pop() with a default and so on, all
because of the unnecessary custom ParamNotFound error which is
never caught in an exception clause at the abc collection on
a higher level.

Signed-off-by: Plamen Dimitrov <[email protected]>
@pevogam pevogam force-pushed the param-not-found-removal branch from 6024638 to 253cc81 Compare June 7, 2019 07:14
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.

1 participant