Skip to content
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

The usage of pendulum in kiota_serialization_json #908

Closed
SamYuen101234 opened this issue Sep 30, 2024 · 2 comments
Closed

The usage of pendulum in kiota_serialization_json #908

SamYuen101234 opened this issue Sep 30, 2024 · 2 comments
Assignees
Labels
Status: No Recent Activity status:waiting-for-author-feedback Issue that we've responded but needs author feedback to close type:bug A broken experience

Comments

@SamYuen101234
Copy link

SamYuen101234 commented Sep 30, 2024

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

@SamYuen101234 SamYuen101234 added status:waiting-for-triage An issue that is yet to be reviewed or assigned type:bug A broken experience labels Sep 30, 2024
@shemogumbe
Copy link
Collaborator

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

microsoft/kiota-serialization-json-python#358

@shemogumbe shemogumbe self-assigned this Oct 1, 2024
@shemogumbe shemogumbe added status:waiting-for-author-feedback Issue that we've responded but needs author feedback to close and removed status:waiting-for-triage An issue that is yet to be reviewed or assigned labels Oct 1, 2024

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: No Recent Activity status:waiting-for-author-feedback Issue that we've responded but needs author feedback to close type:bug A broken experience
Projects
None yet
Development

No branches or pull requests

2 participants