microsoftgraph / msgraph-sdk-python

MIT License
360 stars 50 forks source link

The usage of pendulum in kiota_serialization_json #908

Open SamYuen101234 opened 1 day ago

SamYuen101234 commented 1 day ago

Describe the bug

I am trying to fetch data from sharepoint using msgraph as the following:

result = await ( graph_client .drives.by_drive_id(self.drive_id) .items.by_drive_item_id(self.spreadsheet_id) .workbook.worksheets.by_workbook_worksheet_id(range_name) .used_range.get() )

A cell in my sharepoint excel sheet is in string "now", after fetching with the SDK, the value become an object with type <class 'pendulum.datetime.DateTime'> with the realtime DateTime. This is not what we want the data format, we only need the string without any modification from sharepoint excel.

The problem occurs in function try_get_anything() in json_parse_node.py in kiota_serialization_json

def try_get_anything(self, value: Any) -> Any:
        if isinstance(value, (int, float, bool)) or value is None:
            return value
        if isinstance(value, list):
            return list(map(self.try_get_anything, value))
        if isinstance(value, dict):
            return dict(map(lambda x: (x[0], self.try_get_anything(x[1])), value.items()))
        if isinstance(value, str):
            try:
                datetime_obj = pendulum.parse(value)
                if isinstance(datetime_obj, pendulum.Duration):
                    return datetime_obj.as_timedelta()
                return datetime_obj
            except:
                pass
            try:
                return UUID(value)
            except:
                pass
            return value
        raise ValueError(f"Unexpected additional value type {type(value)} during deserialization.")

I don't know why the design need to parse with pendulum first if the value is string. In most use case of fetching data, we just want to string from excel without modification. Also, pendulum hasn't been update for a long time since 3.0 version.

This problem also occurs before when the string is a four digit number. #653

My suggestion is that removing the parse of pendulum in try_get_anything() and just return pure sting for user.

Expected behavior

"now" as a string in result instead of an object with <class 'pendulum.datetime.DateTime'> type.

How to reproduce

Fetching data from sharepoint excel if the cell is "now"

SDK Version

1.5.4

shemogumbe commented 5 hours ago

Hello @SamYuen101234 thanks for using the SDK and for raising this.

This has been a consistent issue and in the latest patch to the json serialization library, we now only convert ISO 8601 format strings to dates, leaving everything else in its supplied data type, now being a string will thus not be converted to a datetime object.

Is this still a concern with the latest version of the json serialization https://github.com/microsoft/kiota-serialization-json-python/blob/main/kiota_serialization_json/json_parse_node.pylibrary, in the latest version, the code snippet you have attached above is

https://github.com/microsoft/kiota-serialization-json-python/pull/358