-
Notifications
You must be signed in to change notification settings - Fork 96
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
Fix VirtualHostMonster not being able to set mappings under Python 3. #709
Conversation
Johannes Raggam wrote at 2019-10-3 02:35 -0700:
thet commented on this pull request.
> @@ -104,7 +104,7 @@ def set_map(self, map_text, RESPONSE=None):
if hostname not in host_map:
host_map[hostname] = {}
host_map[hostname][port] = pp
- except 'LineError' as msg:
@icemac You've added Products.SiteAccess with this commit.
I don't find the original repo anymore and can't do a git blame there.
What's meant with a ``'LineError'``?
`'LineError'` must be very old -- from a time when Python exceptions have
been strings. I think, modern Python versions no longer support string
exceptions. Should this be true, then there is no longer any way to
raise a `'LineError'` and the `except 'LineError'` could go away.
You may search the `SiteAccess` source for `Error` and try to
find out whether the raising part as meanwhile been modernized
(i.e. the exception transformed from a string to a subclass
of `BaseException`).
|
@d-maurer tnx for the explanation! I couldn't find any references to LineError. IMO Exception is sufficient here, even it's normally bad practice to use such an generic Exception type. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Congratulations to finding and fixing a 15 year old bug!
The PR looks mostly LGTM, I added some change suggestions.
CHANGES.rst
Outdated
@@ -9,6 +9,7 @@ https://github.com/zopefoundation/Zope/blob/4.x/CHANGES.rst | |||
5.0a1 (unreleased) | |||
------------------ | |||
|
|||
- Fix VirtualHostMonster not being able to set mappings under Python 3. Fixes #708 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Fix VirtualHostMonster not being able to set mappings under Python 3. Fixes #708 | |
- Fix VirtualHostMonster not being able to set mappings under Python 3. | |
(`#708 <https://github.com/zopefoundation/Zope/issues/708>`_) | |
@@ -104,7 +104,7 @@ def set_map(self, map_text, RESPONSE=None): | |||
if hostname not in host_map: | |||
host_map[hostname] = {} | |||
host_map[hostname][port] = pp | |||
except 'LineError' as msg: | |||
except Exception as msg: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
except Exception as msg: | |
except ValueError as msg: |
This would restore the original behavior. (See my other comment.)
Additionally the pylint errors should be fixed. |
Fixes #708