Removal of pendulum dependency in kiota-python breaks earlier versions of msgraph-sdk

[chaaaaaarlie]

Describe the bug

We are using msgraph-sdk-python 1.0.0 to automate the download and copying of files from Sharepoint/OneDrive. Until this morning, fields such as created_date_time and modified_date_time were instances of pendulum.datetime.

kiota-python's dependency on pendulum was recenly removed ( https://github.com/microsoft/kiota-python/commit/8a66dc486eccc10d370a2f0e6459b1c91757ddfa ), and a new release (1.8.0) of the various kiota-python artifacts was published this morning. Since msgraph-sdk-python uses imprecise versioning for microsoft-kiota-* (i.e. microsoft-kiota-serialization-json >=1.3.0,<2.0.0"), the newly published version of microsoft-kiota-serialization-json is included with all previous releases of msgraph-sdk-python and our code which relies on created_date_time and modified_date_time being instances of pendulum.datetime breaks despite not updating to a new version of the library.

Expected behavior

Release versions of a publicly available library should use exact versioning for dependencies whenever possible so that changes in the library's dependencies do not break existing versions/releases of the library.

How to reproduce

1. Install version 1.0.0 of msgraph-sdk-python
2. Observe that instances of created_date_time and other datetime fields are instances of datetime.datetime.

SDK Version

1.0.0

Latest version known to work for scenario above?

No response

Known Workarounds

No response

Debug output

Click to expand log

```

</details>


### Configuration

_No response_

### Other information

_No response_

[baywet]

Hi @chaaaaaarlie Thank you for using the SDK and for reaching out.

The type used by the model always was datetime (non pendulum). While I understand that change is a surprise and unpleasant, there was no guarantee being made about the implementation type.

If you need functionality from pendulum, you should be able to perform a conversion with the following snippet.

dt = datetime(2008, 1, 1)
p = pendulum.instance(dt)

(source)

Let us know if you have any additional comments or questions.

[chaaaaaarlie]

Hi @baywet

Thanks for the reply. I'm pretty sure you're mistaken - until the date I created my issue, these objects were delivered to the user with Pendulum datetimes. See below REPL output, where I'm explicitly specifying the previously available version of microsoft-kiota-serialization-json:

$ pip install msgraph-sdk==1.0.0 microsoft-kiota-serialization-json==1.7.1
...
$ python -m asyncio                                                       
asyncio REPL 3.10.14 | packaged by conda-forge | (main, Mar 20 2024, 12:51:49) [Clang 16.0.6 ] on darwin
Use "await" directly instead of "asyncio.run()".
Type "help", "copyright", "credits" or "license" for more information.
>>> import asyncio
>>> from azure.identity import ClientSecretCredential
>>> from msgraph import GraphServiceClient
>>> 
>>> token = 'xxx'
>>> drive_id = 'xxx'
>>> tid = 'xxx'
>>> aid = 'xxx'
>>> secret = 'xxx'
>>> 
>>> cli = GraphServiceClient(
...     ClientSecretCredential(
...         tenant_id=tid,
...         client_id=aid,
...         client_secret=secret,
...     )
... )
>>> 
>>> req = cli.drives.by_drive_id(drive_id).items.by_drive_item_id("root")
>>> response = await (req.delta_with_token(token).get())
>>> dt = response.value[0].file_system_info.created_date_time
>>> print(dt)
2023-11-13 15:23:52+00:00
>>> type(dt)
<class 'pendulum.datetime.DateTime'>
>>>

Same code without specifying the earlier version of microsoft-kiota-serialization-json:

$ pip install --no-cache-dir msgraph-sdk==1.0.0
...
Successfully installed microsoft-kiota-serialization-json-1.9.0 msgraph-sdk-1.0.0
...
$ python -m asyncio                            
...
>>> dt = response.value[0].file_system_info.created_date_time
>>> print(dt)
2023-11-13 15:23:52+00:00
>>> type(dt)
<class 'datetime.datetime'>
>>> 

More to the point, I'm not trying to restore this state - the only reason I have references in my code to these Pendulum objects is so that I can convert them into vanilla datetime.datetime objects.

there was no guarantee being made about the implementation type

This is exactly the problem I'm trying to point out - for a public-facing API library, not guaranteeing the data types you'll receive in an API response, and having those types subsequently change due to an underlying dependency updating, is extremely undesirable behavior. If I use msgraph-sdk 1.0.0 on Tuesday and I get back a different response for the same parameters I used on Monday, it's not a good look. I don't see any reason why a versioned release of this library would not exactly specify the version of all dependencies in use.

Please let me know your thoughts. I think the release strategy for this library needs to be rethought to ensure stability.

Thanks,

Charlie

[baywet]

Thank you for the additional information.

While I understand this change can be destabilizing I don't think our team is going to make that additional guarantee in the future unless we get overwhelming feedback from customers it's needed. Please understand that while I'm going to provide more details below, I do acknowledge those kinds of changes are harder to catch. And we should probably at least have done a better job at outlining the change in the changelog.

It'd make the SDK too rigid and wouldn't allow us to swap encapsulated implementation types as needed.

The API surface type guarantee to prevent source breakages is BCL datetime. And that was maintained through the change.

Since pendulum datetime inherits from BCL datetime all that was "lost" was the type in the hierarchy as well as additional methods and behaviours from that type.

But again, the API surface never guaranteed it'd return that type to begin with.

If applications have a strong dependency on that type, they should have tested/converted returned values from the SDK API surface, added a direct dependency etc...

Let me know if you have additional questions or concerns here.

[chaaaaaarlie]

If applications have a strong dependency on that type, they should have tested/converted returned values from the SDK API surface, added a direct dependency etc...

A DriveItem can have hundreds of attributes - I'd prefer not to have to isinstance every single reference to one just in case a string unexpectedly stops being a string one day.

It'd make the SDK too rigid and wouldn't allow us to swap encapsulated implementation types as needed.

Changing implementation types is a great idea, especially when it removes a dependency as in this case. The problem is that it should be done by creating a new release of msgraph-sdk-python, not by making unannounced and undetectable alterations to the operation of existing releases. It took hours of our time just to identify the cause of this issue.

[baywet]

Thanks for the feedback.

While the change was outlined in the kiota libraries changelog, it was obscured by the transitive dependency change for graph SDK consumers.

And this change didn't mandate a major version bump as it's not part of what's considered as a source breaking change.

We'll try to be more careful assessing the potential impact for such changes in the future!

Let us know if you have any additional comments or questions.

[chaaaaaarlie]

If this isn't considered a source breaking change, then why did I wake up one day to find my source broken?

New versions of msgraph-sdk-python seem to be released on a weekly basis. Why should it be the user's responsibility to manage updates for transitive dependencies?

Thank you for providing an excellent example I can use when I recommend clients away from Microsoft platforms.

[sanmai-NL]

@chaaaaaarlie: What prevented you from using a type checker?

[svrooij]

If I may summarize this issue:

• Graph SDK uses pretty loose dependencies
• By installing one version of this SDK, you get something different depending on which dependency version is available, which might be undesirable
• where the SDK said it would return a python datetime it was actually returning a pendulum.datetime which was a subclass of the native datetime.
• with the removal of pendulum, the type-check for pendulum.datetime does no longer work

Let’s make this issue about wether or not the graph SDK should force strict versions for dependencies. I don’t have an opinion about this, and I cannot find any good resources on making this choice. But I must agree that is you install version x of this SDK, it should always behave the same.

[sanmai-NL]

If something like Renovate bot is used, more version-constrained deps can be used without falling out of date.

[sanmai-NL]

@svrooij Here's an article describing principles and best practices, also applicable to Python.

https://docs.renovatebot.com/dependency-pinning/

I think generally, programs should have fixed dependencies, being dependency DAG 'roots'. Libraries like msgraph, though, should strive to be compatible with as many dependers/dependents as possible and therefore not fixate dependencies.