-
Notifications
You must be signed in to change notification settings - Fork 441
Add xsd date type #602
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
Add xsd date type #602
Conversation
Codecov Report
@@ Coverage Diff @@
## master #602 +/- ##
==========================================
- Coverage 65.64% 65.63% -0.01%
==========================================
Files 103 103
Lines 25614 25615 +1
==========================================
- Hits 16814 16813 -1
- Misses 8800 8802 +2
Continue to review full report at Codecov.
|
668fbf4
to
3aa4923
Compare
@@ -191,6 +192,11 @@ def _wrong_type_value(xsd, value): | |||
'to_type': _str, | |||
'to_text': _str, | |||
}, | |||
'date': { | |||
'type': datetime.date, | |||
'to_type': lambda x: datetime.datetime.strptime(x, '%Y-%m-%d').date(), |
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 is setting a specific time format, whereas xsd:date
allows for [-]CCYY-MM-DD[Z\|(+\|-)hh:mm]
. I'll probably do #518 first, then point this to the new datetime parser (saml2.datetime.parse
) that takes that into account, as well as timezones.
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.
A better generalization on top of #518 would be the best solution for taking also datetime as timestamps, with timezone support.
```` <saml:Attribute Name="dateOfBirth"> <saml:AttributeValue xsi:type="xs:date">2015-12-06</saml:AttributeValue> </saml:Attribute> ```` Otherwise I get an ValueError Exception like ```` Type and value do not match: date:<class 'str'>:2015-12-06 ```` because `type(value) is not valid_type == True` I didn't write any unit test, just tested in my context. I also think that the following code https://github.com/IdentityPython/pysaml2/pull/602/files#diff-6c156669cad61eda35e679329251dce9R197 could be also improved according to IdentityPython#518 if you agree.
3aa4923
to
26e8303
Compare
@c00kiemon5ter waiting for a better world (and refactored one!) consider to merge this as it is and open an issue to have track of its weakness. We don't have a clear roadmap about when the datetime handlers in pysaml2 will be updated and this xsd:type is blocking for many implementations, I believe |
Closed by d88a979 |
This PR is needed for returning attributes like
Otherwise I get an ValueError Exception like
because
type(value) is not valid_type == True
I didn't write any unit test, just tested in my context.
I also think that the following code https://github.com/IdentityPython/pysaml2/pull/602/files#diff-6c156669cad61eda35e679329251dce9R197 could be also improved according to #518 if you agree.