-
Notifications
You must be signed in to change notification settings - Fork 166
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
switch to suds-community #192
Conversation
cc: @qitia |
thanks for the PR? may I know why you choose this version(1.00b1) instead of 0.8.5? |
@@ -11,7 +11,7 @@ | |||
history = f.read().replace('.. :changelog:', '') | |||
|
|||
requirements = [ | |||
'suds-jurko==0.6.0', | |||
'suds-community>=1.0.0b1', |
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.
why this version instead of 0.8.5
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.
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.
thanks.
While test locally, I found there is some difference between jurko version and the community version when build an object. in jurko version, a complex attribute is created regardless its occurrence count and optional or not.
Jurko version
setattr(data, type.name, value)
Community version
setattr(data, type.name, value if not type.optional() or type.multi_occurrence() else None)
I am not sure if there is customer already depends on this jurko feature, if we upgrade to community, this will break those customers. any idea?
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.
I don't see any issue with this as a customer of Bing. I think this will be fine. Also could really use this fix, as it's blocking fresh installs.
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.
This also break our bulk mapping. Sample code and here: the entity
is null with this sudo-community implementation and will bring trouble.
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.
Do you have a runnable example or a test case?
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.
try this exmple.
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.
Please see this issue for more background on why this was changed: suds-community/suds#14, however, that should have reverted the behavior to the 0.6.0 behavior.
Hi there! Any plan on moving forward with this PR? It's impacting us also. |
we've published 13.0.11.1. please give it a try. |
Does this pr need closing? |
yes we could close this PR without merge. |
Fixes #191