Skip to content

Remove deprecated datetime functions - #1134

Merged
Fokko merged 4 commits into
apache:mainfrom
hussein-awala:deprecated_datetime
Nov 19, 2024
Merged

Remove deprecated datetime functions#1134
Fokko merged 4 commits into
apache:mainfrom
hussein-awala:deprecated_datetime

Conversation

@hussein-awala

Copy link
Copy Markdown
Member

closes: #1133

@kevinjqliukevinjqliu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

Comment threadpyiceberg/table/__init__.py Outdated

snapshots.append({
"committed_at": datetime.utcfromtimestamp(snapshot.timestamp_ms / 1000.0),
"committed_at": datetime.fromtimestamp(snapshot.timestamp_ms / 1000.0, tz=timezone.utc),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comment threadpyproject.toml
"ignore:unclosed <socket.socket",
# Remove this in a future release of PySpark.
"ignore:distutils Version classes are deprecated. Use packaging.version instead.",
"ignore:datetime.datetime.utcnow\\(\\) is deprecated and scheduled for removal in a future version.",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

huh, not sure where this went, theres no references in the codebase
https://github.com/search?q=repo%3Aapache%2Ficeberg-python%20utcnow&type=code

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

it might be used in one of the dependency packages, but we don't need to filter the warning in that case

@FokkoFokko left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for cleaning this up @hussein-awala Could you resolve the conflicts? Thanks!

@kevinjqliukevinjqliu added this to the PyIceberg 0.9.0 release milestone Oct 30, 2024
@hussein-awala

Copy link
Copy Markdown
MemberAuthor

Thanks for cleaning this up @hussein-awala Could you resolve the conflicts? Thanks!

I did and looks like the usage of the deprecated method was removed by another PR, this one now just removes the useless warning.

@kevinjqliu

Copy link
Copy Markdown
Contributor

https://github.com/search?q=repo%3Aapache%2Ficeberg-python%20utcfromtimestamp&type=code
there's a few instances of utcfromtimestamp

@kevinjqliukevinjqliu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! Thanks for working on this.

Can you update the PR to reflect that this is only removing 1 out of 2 functions?
And open partially close #1133

Perhaps we can also link #1133 as a comment above the filter warning

@Fokko

Copy link
Copy Markdown
Contributor

@kevinjqliu Let's move this forward for now. Thanks @hussein-awala for fixing this 👍

@Fokko
Fokko merged commit 5f0f770 into apache:mainNov 19, 2024
@kevinjqliukevinjqliu mentioned this pull request Nov 27, 2024
sungwy pushed a commit to sungwy/iceberg-python that referenced this pull request Dec 7, 2024
* Remove deprecated datetime functions
* remove all usage of deprecated methods
* readd utcnow filter because of boto3
sungwy pushed a commit to sungwy/iceberg-python that referenced this pull request Dec 7, 2024
* Remove deprecated datetime functions
* remove all usage of deprecated methods
* readd utcnow filter because of boto3
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove deprecated datetime functions

3 participants

@hussein-awala@kevinjqliu@Fokko