Skip to content

chore: remove pyrfc3339 and change to datetime.datetime.fromisoformat… #1247

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Feb 5, 2025
Merged

chore: remove pyrfc3339 and change to datetime.datetime.fromisoformat… #1247

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions juju/client/gocookies.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import json
import time

import pyrfc3339
from backports.datetime_fromisoformat import datetime_fromisoformat


class GoCookieJar(cookiejar.FileCookieJar):
Expand Down Expand Up @@ -52,7 +52,7 @@ def go_to_py_cookie(go_cookie):
"""Convert a Go-style JSON-unmarshaled cookie into a Python cookie"""
expires = None
if go_cookie.get("Expires") is not None:
t = pyrfc3339.parse(go_cookie["Expires"])
t = datetime_fromisoformat(go_cookie["Expires"])
expires = t.timestamp()
return cookiejar.Cookie(
version=0,
Expand Down Expand Up @@ -101,8 +101,9 @@ def py_to_go_cookie(py_cookie):
if py_cookie.path_specified:
go_cookie["Path"] = py_cookie.path
if py_cookie.expires is not None:
unix_time = datetime.datetime.fromtimestamp(py_cookie.expires)
# Note: fromtimestamp bizarrely produces a time without
# a time zone, so we need to use accept_naive.
go_cookie["Expires"] = pyrfc3339.generate(unix_time, accept_naive=True)
go_cookie["Expires"] = datetime.datetime.fromtimestamp(
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the output exactly same?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not exactly, these are the outputs:
pyrfc3339: 2025-01-30T14:49:41Z
datetime.isoformat(): 2025-01-30T14:49:41.562687

But this was already discussed, I think in the other PR about the same issue, but as both are different formats that are part of the RFC3339 specification, we decided to proceed.

But if it's not possible, I can find another solution. There was also that manual parse I did to make them equal.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's just something I need to be clear about and run by juju (server) folks.

@SimonRichardson wdyt? or nominate someone, pls.

Copy link
Member

Choose a reason for hiding this comment

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

My understanding is that Go uses RFC1123 ie "Mon, 02 Jan 2006 15:04:05 MST" as the expires time serialisation format. I'm not quite sure how RFC3339 worked TBH.
Regardless, the current best practice is to not use expiry time and instead use max age. I wonder if we should start to use that here as well.

py_cookie.expires
).isoformat()
return go_cookie
6 changes: 3 additions & 3 deletions juju/machine.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import logging
import typing

import pyrfc3339
from backports.datetime_fromisoformat import datetime_fromisoformat

from juju.utils import block_until, juju_ssh_key_paths

Expand Down Expand Up @@ -239,7 +239,7 @@ def agent_status(self):
@property
def agent_status_since(self):
"""Get the time when the `agent_status` was last updated."""
return pyrfc3339.parse(self.safe_data["agent-status"]["since"])
return datetime_fromisoformat(self.safe_data["agent-status"]["since"])

@property
def agent_version(self):
Expand All @@ -266,7 +266,7 @@ def status_message(self):
@property
def status_since(self):
"""Get the time when the `status` was last updated."""
return pyrfc3339.parse(self.safe_data["instance-status"]["since"])
return datetime_fromisoformat(self.safe_data["instance-status"]["since"])

@property
def dns_name(self):
Expand Down
6 changes: 3 additions & 3 deletions juju/unit.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

import logging

import pyrfc3339
from backports.datetime_fromisoformat import datetime_fromisoformat

from juju.errors import JujuAPIError, JujuError

Expand All @@ -27,7 +27,7 @@ def agent_status(self):
@property
def agent_status_since(self):
"""Get the time when the `agent_status` was last updated."""
return pyrfc3339.parse(self.safe_data["agent-status"]["since"])
return datetime_fromisoformat(self.safe_data["agent-status"]["since"])

@property
def is_subordinate(self):
Expand All @@ -54,7 +54,7 @@ def workload_status(self):
@property
def workload_status_since(self):
"""Get the time when the `workload_status` was last updated."""
return pyrfc3339.parse(self.safe_data["workload-status"]["since"])
return datetime_fromisoformat(self.safe_data["workload-status"]["since"])

@property
def workload_status_message(self):
Expand Down
4 changes: 2 additions & 2 deletions juju/user.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

import logging

import pyrfc3339
from backports.datetime_fromisoformat import datetime_fromisoformat

from . import errors, tag
from .client import client
Expand Down Expand Up @@ -31,7 +31,7 @@ def display_name(self):

@property
def last_connection(self):
return pyrfc3339.parse(self._user_info.last_connection)
return datetime_fromisoformat(self._user_info.last_connection)

@property
def access(self):
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ classifiers = [
]
dependencies = [
"macaroonbakery>=1.1,<2.0",
"pyRFC3339>=1.0,<2.0",
"pyyaml>=5.1.2",
"websockets>=13.0.1",
"paramiko>=2.4.0",
Expand All @@ -35,6 +34,7 @@ dependencies = [
"packaging",
"typing-extensions>=4.5.0",
'backports.strenum>=1.3.1; python_version < "3.11"',
"backports-datetime-fromisoformat>=2.0.2",
]
[project.optional-dependencies]
dev = [
Expand Down
2 changes: 1 addition & 1 deletion setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
package_data={"juju": ["py.typed"]},
install_requires=[
"macaroonbakery>=1.1,<2.0",
"pyRFC3339>=1.0,<2.0",
"pyyaml>=5.1.2",
"websockets>=13.0.1",
"paramiko>=2.4.0",
Expand All @@ -33,6 +32,7 @@
"packaging",
"typing-extensions>=4.5.0",
'backports.strenum>=1.3.1; python_version < "3.11"',
"backports-datetime-fromisoformat>=2.0.2",
],
extras_require={
"dev": [
Expand Down
5 changes: 3 additions & 2 deletions tests/unit/test_gocookies.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,14 @@
import unittest
import urllib.request

import pyrfc3339
from backports.datetime_fromisoformat import datetime_fromisoformat

from juju.client.gocookies import GoCookieJar

# cookie_content holds the JSON contents of a Go-produced
# cookie file (reformatted so it's not all on one line but
# otherwise unchanged).

cookie_content = """
[
{
Expand Down Expand Up @@ -223,7 +224,7 @@ def test_expiry_time(self):
]"""
jar = self.load_jar(content)
got_expires = tuple(jar)[0].expires
want_expires = int(pyrfc3339.parse("2345-11-15T18:16:08Z").timestamp())
want_expires = int(datetime_fromisoformat("2345-11-15T18:16:08Z").timestamp())
self.assertEqual(got_expires, want_expires)

def load_jar(self, content):
Expand Down
Loading