Repository navigation
Remove xmltodict and replace with Python's built-in xml ElementTree - #190
Open
andrewhughes101 wants to merge 1 commit into
Open
andrewhughes101 wants to merge 1 commit into
andrewhughes101 wants to merge 1 commit into
Conversation
andrewhughes101
force-pushed
the
xmltodict-removal
branch
2 times, most recently
from
December 15, 2025 18:40
4188759 to
0092770
Compare
andrewhughes101
marked this pull request as ready for review
December 15, 2025 18:41
andrewhughes101
force-pushed
the
xmltodict-removal
branch
from
December 23, 2025 17:53
0092770 to
a50e0cb
Compare
andrewhughes101
force-pushed
the
xmltodict-removal
branch
from
December 24, 2025 11:01
a50e0cb to
0f8dcb9
Compare
ledina
reviewed
Apr 13, 2026
Comment on lines
178
to
180
| # Xmltodict parses inner error types as Dicts when there is only one item in | ||
| # it even though it may well be a list if multiple results were returned. If | ||
| # we find a dict here, wrap it in a list so we can account for only one |
Collaborator
There was a problem hiding this comment.
Probably need to change this comment too
ledina
reviewed
Apr 13, 2026
ledina
reviewed
Apr 13, 2026
| version="3.0" connect_version="0610"> | ||
| <resultsummary api_response1="1024" api_response2="0" api_response1_alt="OK" api_response2_alt="" recordcount="4" displayed_recordcount="4"/> | ||
| <records> | ||
| <cicslocalfile _keydata="BLAH" accessmethod="VSAM" add="ADDABLE" addcnt="0" basdefinever="0" basedsname="DFHCSD" blockformat="BLOCKED" |
Collaborator
There was a problem hiding this comment.
Would it be better to have 4 records, to match recordcount, or maybe make it 2 so we can test working with more than one?
ledina
reviewed
Apr 13, 2026
Comment on lines
+99
to
+102
| if text: | ||
| result['#text'] = text if result else None | ||
| return OrderedDict([(tag, result if result else text)]) | ||
| return OrderedDict([(tag, result if result else None)]) |
Collaborator
There was a problem hiding this comment.
Would an if result start make this a bit easier to read?
Suggested change
| if text: | |
| result['#text'] = text if result else None | |
| return OrderedDict([(tag, result if result else text)]) | |
| return OrderedDict([(tag, result if result else None)]) | |
| if result: | |
| result['#text'] = text if text else None | |
| return OrderedDict([(tag, result)]) | |
| if text: | |
| return OrderedDict([(tag, text)]) | |
| return OrderedDict([(tag, None)]) |
ledina
reviewed
Apr 13, 2026
ledina
reviewed
Apr 13, 2026
ledina
requested changes
Apr 13, 2026
ledina
reviewed
Apr 13, 2026
AndrewTwydell
approved these changes
Sep 14, 2026
AndrewTwydell
left a comment
AndrewTwydell
left a comment
Collaborator
There was a problem hiding this comment.
Think this import needs updating, but I've run tests and installed / run manually and CMCI modules work as expected.
|
|
||
| __metaclass__ = type | ||
|
|
||
| from typing import OrderedDict |
Collaborator
There was a problem hiding this comment.
Suggested change
| from typing import OrderedDict | |
| from collections import OrderedDict |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Built off the other PR to add arch and dev guide docs
Needs some unit testing around the new util explicitly but(Added unit tests) works nicely with existing ones and integration tests pass locally 🎉