8000 gh-107428: Added overridable methods for handling duplicate sections and options in ConfigParser. by Hyperclaw79 · Pull Request #107430 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

gh-107428: Added overridable methods for handling duplicate sections and options in 8000 ConfigParser. #107430

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

gh-107428: Added overridable methods for handling duplicate sections and options in ConfigParser. #107430

wants to merge 7 commits into from

Conversation

Hyperclaw79
Copy link
@Hyperclaw79 Hyperclaw79 commented Jul 29, 2023 •
edited by bedevere-bot
Loading

@bedevere-bot bedevere-bot mentioned this pull request Jul 29, 2023
Open
2 tasks
@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@ghost
Copy link
ghost commented Jul 29, 2023 •
edited by ghost
Loading

All commit authors signed the Contributor License Agreement.
CLA signed

@serhiy-storchaka serhiy-storchaka mentioned this pull request Jul 30, 2023
Open
@serhiy-storchaka
Copy link
Member

See also #56871, #65528 and #75709. I think they all are parts of the same problem. #95546 is also related -- if duplicated sections or options are forbidden, they should not be changed before raising an exception.

Simply adding underscored methods to raise exceptions is not a good solution of your problem. Because these methods is not a part of the public API, they will likely be changed or removed in more general solution of the above issues.

@serhiy-storchaka serhiy-storchaka mentioned this pull request Jul 30, 2023
Open
@Hyperclaw79
Copy link
Author
Hyperclaw79 8000 commented Jul 30, 2023

I have read those issues before. The main difference in context here is that instead of suggesting how to reimplement the existing design, here we can keep the default behaviour while providing the option for the user to tweak it according to their business logic.
But I do agree with your argument about the private method being overridden being an antipattern. Since the _read method itself is a private method, I wasn't sure about the best route to take.
I wasn't aware that there's a plan to implement a new general solution. If it also involves the ability for the users to add custom business logic like logging a warning, then I think this PR can be closed.

@serhiy-storchaka
Copy link
Member

For now, you can override _read(), and it will work for you in Python 3.12 and older versions. It is unlikely it will be broken in a bugfix. But you need to test it with newer versions. On other hand, overriding _handle_duplicate_option() will not help you until you drop support of Python 3.12 and older versions. At that time there is a chance to get a proper solution.

If you want a solution which does not depend on Python version, the only way is to copy the code in your project or use a third-party package which implements your requirements.

@Hyperclaw79
Copy link
Author

The points about Python versions are all valid.
Regarding the overriding of _read() method, the main drawback is the repetition of lot of number of lines just to add two log lines. But yeah, like I've mentioned before, if a general solution is in the roadmap, that would be better.

@Hyperclaw79 Hyperclaw79 requested a review from jaraco as a code owner June 14, 2024 14:39
@Hyperclaw79
Copy link
Author

Since overriding private methods directly is not the best standard, an alternative approach I'm thinking about is allowing two new parameters during init of RawConfigParser. They will accept functions and default to lambda: raise Duplicate(Option/Section)Error. We can use these in place of _handle_duplicate_(option/section).
If anyone has any comments about this, please let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jaraco jaraco Awaiting requested review from jaraco jaraco is a code owner

Assignees
No one assigned
Labels
awaiting review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Hyperclaw79 @bedevere-bot @serhiy-storchaka
0