Skip to content

Conversation

@dgarros
Copy link
Collaborator

@dgarros dgarros commented Feb 23, 2025

Related to opsmill/infrahub-sdk-python#256

This PR replaces the library pendulum which is not maintained anymore with whenever for datetime management.
In the process, I took the opportunity to use our own Timestamp class in more places instead of using a third party library, this will help the migration to another library in the future (I hope we don't have too)

@github-actions github-actions bot added the group/backend Issue related to the backend (API Server, Git Agent) label Feb 23, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented Feb 23, 2025

CodSpeed Performance Report

Merging #5820 will not alter performance

Comparing dga-20250223-pendulum-whenever (e149a20) with release-1.2 (1f077ba)

Summary

✅ 10 untouched benchmarks

@dgarros dgarros force-pushed the dga-20250223-pendulum-whenever branch from f7b8e63 to 16f3e39 Compare February 24, 2025 09:10
@dgarros dgarros changed the base branch from develop to release/1.2 February 24, 2025 18:22
@dgarros dgarros force-pushed the dga-20250223-pendulum-whenever branch from 16f3e39 to 2f3b86f Compare February 24, 2025 18:24
@dgarros dgarros changed the base branch from release/1.2 to release-1.2 February 25, 2025 07:33
Base automatically changed from release-1.2 to develop February 25, 2025 14:05
@dgarros dgarros changed the base branch from develop to release-1.2 February 26, 2025 06:27
@dgarros dgarros force-pushed the dga-20250223-pendulum-whenever branch from 5831b5e to b766d46 Compare February 26, 2025 07:03
@dgarros dgarros marked this pull request as ready for review February 26, 2025 09:06
@dgarros dgarros requested a review from a team as a code owner February 26, 2025 09:06
Copy link
Contributor

@ogenstad ogenstad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some comments but looks good to me.

async def to_graphql(self, *args: Any, **kwargs: Any) -> DateTime: # noqa: ARG002
return self.obj
async def to_graphql(self, *args: Any, **kwargs: Any) -> datetime: # noqa: ARG002
return self.to_datetime()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this method used anywhere?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

from neo4j.graph import Path as Neo4jPath
from neo4j.graph import Relationship as Neo4jRelationship
from pendulum import Interval
from whenever import TimeDelta
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to get rid of this import as well, if we can map it to a TimeDelta from our own TimeStamp class.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed but it sounds like a bigger effort


@property
def time_range(self) -> Interval:
def time_range(self) -> TimeDelta:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per the updates in the SDK the time_range here and on line 420 will raise a deprecation warning due to us keeping the code in the function intact.

return self.to_time.obj - self.from_time.obj

We don't want to use ._obj instead but perhaps we need some other method. Or some dundermethods on our own class so that it's possible to instead run:

return self.to_time - self.from_time

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, I fixed it

@dgarros dgarros force-pushed the dga-20250223-pendulum-whenever branch from b766d46 to eaefaaa Compare February 27, 2025 15:57
@dgarros dgarros force-pushed the dga-20250223-pendulum-whenever branch from eaefaaa to e149a20 Compare February 27, 2025 17:34
@dgarros dgarros merged commit a15a808 into release-1.2 Feb 27, 2025
34 checks passed
@dgarros dgarros deleted the dga-20250223-pendulum-whenever branch February 27, 2025 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

group/backend Issue related to the backend (API Server, Git Agent)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants