Skip to content
This repository has been archived by the owner on Jul 18, 2024. It is now read-only.

Library works from Python 2.7+ (Compatible with all 3.x versions) #9

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

dslapelis
Copy link

@dslapelis dslapelis commented Jan 25, 2019

This change is Reviewable

@dslapelis dslapelis mentioned this pull request Jan 25, 2019
@dslapelis
Copy link
Author

@Tzur-i can I get a review on this please?

@dslapelis dslapelis closed this Jan 31, 2019
@dslapelis dslapelis reopened this Jan 31, 2019
@@ -24,7 +24,7 @@

import json
from socket import getfqdn
from bunch import Bunch
from munch import Munch
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why Munch?
A Replacement like this requires extensive testing.
What's the benefit of this?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe that Bunch is still supported, and it is incompatible with Python 3.x. All of my tests failed with Bunch in Python 3. It appears that Infinidat forked Bunch and created Munch.

https://github.com/Infinidat/munch

@@ -189,7 +194,7 @@ class TerminationDetectingXMLParser(object):

def __init__(self):
self.tree_builder = _TerminationDetectingTreeBuilder()
self.xml_tree_builder = et.XMLTreeBuilder(target=self.tree_builder)
self.xml_tree_builder = et.XMLParser(target=self.tree_builder)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

XMLParser will also require extensive testing. I know that TreeBuilder is old, but what's the benefit?

Copy link
Author

@dslapelis dslapelis Jan 31, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

XMLTreeBuilder did not appear to exist in Python 3. XMLParser should be used instead; I can add the backwards compatibility try/except stated in the documentation http://effbot.org/elementtree/elementtree-xmlparser.htm

@tzurE
Copy link
Contributor

tzurE commented Feb 3, 2019

Hi,
Thanks for your contribution!
Let us test your suggested changes and verify them and we'll update you as soon as we can continue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants