Skip to content

Only fixes dates for min date, not for all other wrong dates < 15.10.1582 #263

Closed

Description

@manuelseeger

More of a discussion topic than a bug. Python-pyodata has a "dirty" fix for SAP's faulty unix timestamp implementation.

def ms_since_epoch_to_datetime(value, tzinfo):

I am wondering why this was not raised to the service provider: SAP standard JSON serialization. The problem is not limited to 0001-01-01, every date below 15.10.1582 is serialized wrong because the JSON implementation doesn't account for ABAP data type d's rather peculiar behavior of mixing Julian and Gregorian calendar.

This should really be fixed by SAP on the provider level rather than having every client library built in dirty hacks like this to not run into errors (while still receiving wrong dates!).

A proper fix on client side would have to implement this entire conversion to account for SAP's faulty timestamps:
https://en.wikipedia.org/wiki/Conversion_between_Julian_and_Gregorian_calendars
But feel free to close if you feel this is out of scope.

Activity

  1. phanak-sap commented on Nov 29, 2023

    @phanak-sap
    Contributor

    Hi @manuelseeger

    I do not know what to respond here. The library was initially build around what we see, for the purpose of writing integration tests for out teams Fiori apps - because existing python libraries were not covering what we needed or not maintained anymore. Then later the pyodata was open-source for the community usage for generic odata v2 usage and maintained with bugs incoming from enhancing the scope in such way.

    Feel free to create a customer incident for SAP that "15.10.1582 is serialized wrong" - my wild guess as person (not speaking for SAP as a company) is that the relevant code is stabilized for such long time, that it would be much more burden to actually fix this, probably would break some backward compatibility in the product and yet, as a basically ERP and Accounting to be the core functionality there, customers probably do not have invoices from the 16th century and before.

    But that function is fixing problem about timezones and Edm.DateTimeOffset not working properly. Check out PR #184 and commit message for ec56995. That's why I think it is correct to be present in the core model.py and not vendor/SAP.py.

    Do you see anything that should be fixed on the pyodata code side?

  2. manuelseeger commented on Nov 29, 2023

    @manuelseeger
    ContributorAuthor

    Don't get me wrong I am not saying this library has a bug. And I understand the reasoning.

    I also don't expect SAP will fix the messed up date serializing at this point. Although an alternative, fixed serializer which can be invoked by a custom header or something like that would be great.
    And yes, dates before 1582 are probably not business relevant in most cases.

    Still, that leaves client libraries to implement hacks to not get out of bounds errors for 0001-01-01, like pyodata does. I had to do the same in other client libraries and it's pretty painful for libraries that you don't control yourself.

    I guess I was just wondering if that was a known problem before since I saw the same workaround that I had to build into other libraries here.

  3. phanak-sap commented on Nov 29, 2023

    @phanak-sap
    Contributor

    OK, seems we are on the same boat here in same storm :)

    Closing the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    questionFurther information is requested

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions