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

ncclient should require minimum of lxml>=3.3.0 #84

Closed
wants to merge 1 commit into from

Conversation

bdemers
Copy link

@bdemers bdemers commented Aug 26, 2015

This test is looks ugly to me, so it should probably not be merged, without a little more massaging.
The actual fix is bumping the version in requirements.txt

...

Added test that reproduces problem when using lxml<3.3.0
When sending multiple messages the state of the attributes bleeds into the next message.

This was fixed in lxml-3.3.0beta4
Originally reported: https://bugzilla.redhat.com/show_bug.cgi?id=1242011

@leopoul
Copy link
Member

leopoul commented Jan 30, 2016

Fixed in 0.4.7 https://github.com/ncclient/ncclient/tree/v0.4.7

thnx for pointing it out

@leopoul
Copy link
Member

leopoul commented Feb 14, 2016

Hi Brian,
Maybe we should review the lxml req.

Some days ago, Thomas pointed out that the lxml version req could be lowered: #109.

In the meantime I am trying to run your unittest locally but I fail to do so - maybe I have missed something... :

python -m unittest test_clear_attributes

DEBUG:ncclient.transport.session:<Session(session, initial daemon)> created: client_capabilities=<ncclient.devices.nexus.NexusDeviceHandler object at 0x10510c210>
DEBUG:ncclient.transport.session:installing listener <ncclient.operations.rpc.RPCReplyListener object at 0x10510cd10>
INFO:ncclient.operations.rpc:Requesting 'Get'
DEBUG:ncclient.operations.rpc:Async request, returning <ncclient.operations.retrieve.Get object at 0x10510cdd0>
F
======================================================================
FAIL: test_clear_attributes_from_previous_message (test_clear_attributes.Test_LXML_problem)
test sending multiple messages results in valid xml
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test_clear_attributes.py", line 40, in test_clear_attributes_from_previous_message
    assert_equal(self._read_test_string('test/expected_get_message.xml'), xml_string)
AssertionError: '<?xml version="1.0" encoding="UTF-8"?><rpc xmlns:vlan_mgr_cli="http://www.cisco.com/nxos:1.0:vlan_mgr_cli" xmlns:nfcli="http://www.cisco.com/nxos:1.0:nfcli" xmlns:nxos="http://www.cisco.com/nxos:1.0" xmlns:if="http://www.cisco.com/nxos:1.0:if_manager" xmlns="urn:ietf:params:xml:ns:netconf:base:1.0" message-id="urn:uuid:UUID-HERE"><get><filter type="subtree"><cmd>show version</cmd></filter></get></rpc>\n' != '<?xml version="1.0" encoding="UTF-8"?><rpc xmlns:if="http://www.cisco.com/nxos:1.0:if_manager" xmlns:nfcli="http://www.cisco.com/nxos:1.0:nfcli" xmlns:nxos="http://www.cisco.com/nxos:1.0" xmlns:vlan_mgr_cli="http://www.cisco.com/nxos:1.0:vlan_mgr_cli" xmlns="urn:ietf:params:xml:ns:netconf:base:1.0" message-id="urn:uuid:UUID-HERE"><get><filter type="subtree"><cmd>show version</cmd></filter></get></rpc>'

----------------------------------------------------------------------
Ran 1 test in 0.030s

FAILED (failures=1)

I have tested with lxml-3.2.1 , 3.3.6, 3.5.0

Thnx in advance,

Best,
Leo

@leopoul leopoul reopened this Feb 14, 2016
@leopoul
Copy link
Member

leopoul commented Feb 14, 2016

Also, it would help if you could include your test in ncclient's unit tests

Thnx again,

Best,
Leo

@bdemers
Copy link
Author

bdemers commented Feb 15, 2016

NOTE: Force pushed updated to this branch

@@ -0,0 +1 @@
<?xml version="1.0" encoding="UTF-8"?><rpc xmlns:if="http://www.cisco.com/nxos:1.0:if_manager" xmlns:nfcli="http://www.cisco.com/nxos:1.0:nfcli" xmlns:nxos="http://www.cisco.com/nxos:1.0" xmlns:vlan_mgr_cli="http://www.cisco.com/nxos:1.0:vlan_mgr_cli" xmlns="urn:ietf:params:xml:ns:netconf:base:1.0" message-id="urn:uuid:UUID-HERE"><get><filter type="subtree"><cmd>show version</cmd></filter></get></rpc>
Copy link
Author

Choose a reason for hiding this comment

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

the xmlns order changed between now, and the last time I messed with this. I'm not sure the best way to deal with this.

Should the resulting xml be parsed, and the object diff'd ?
I don't want to ignore all the attributes in the root node because that is where the problem was with lxml 3.2.1

Thoughts / ideas ?

@einarnn
Copy link
Contributor

einarnn commented Jul 1, 2018

The requirement is now lxml>=3.3.0. This PR now seems redundant, closing.

@einarnn einarnn closed this Jul 1, 2018
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.

None yet

3 participants