Skip to content

refactor: change use of pyrfc3339 to use ciso8601 #1243

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

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
27 changes: 24 additions & 3 deletions juju/client/gocookies.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import json
import time

import pyrfc3339
import ciso8601


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 = ciso8601.parse_rfc3339(go_cookie["Expires"])
expires = t.timestamp()
return cookiejar.Cookie(
version=0,
Expand Down Expand Up @@ -104,5 +104,26 @@ def py_to_go_cookie(py_cookie):
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"] = generate_rfc3339_from_unix_time(unix_time)
Copy link
Contributor

Choose a reason for hiding this comment

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

I see that there's a solid-looking set of tests in tests/unit/test_gocookies.py.
Could you clarify why the custom parses function was needed, what specific inputs could not be parsed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ciso8601 does not have a function to generate a rfc3339 timestamp from a datetime, as this one of the pyrfc3339 library, so I implemented my own.

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if datetime.isoformat() would be enough.
I'll need someone who knows about go-style cookies and why are those important, if Juju relies on those in some way.

return go_cookie


def generate_rfc3339_from_unix_time(unix_time: datetime.datetime) -> str:
splitted_rfc = unix_time.isoformat().split(".")
if len(splitted_rfc) == 1:
return f"{splitted_rfc[0]}Z"
rfc, discard = splitted_rfc
discard = discard.split("+")
if len(discard) > 1:
rfc = datetime.datetime.fromisoformat(rfc)
hours, minutes = discard[1].split(":")
rfc -= datetime.timedelta(hours=int(hours), minutes=int(minutes))
rfc = rfc.isoformat()
else:
discard = discard[0].split("-")
if len(discard) > 1:
rfc = datetime.datetime.fromisoformat(rfc)
hours, minutes = discard[1].split(":")
rfc += datetime.timedelta(hours=int(hours), minutes=int(minutes))
rfc = rfc.isoformat()
return f"{rfc}Z"
6 changes: 3 additions & 3 deletions juju/machine.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import logging
import typing

import pyrfc3339
import ciso8601

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 ciso8601.parse_rfc3339(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 ciso8601.parse_rfc3339(self.safe_data["instance-status"]["since"])

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

import logging

import pyrfc3339
import ciso8601

from juju.errors import JujuAPIError, JujuError

Expand All @@ -25,7 +25,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 ciso8601.parse_rfc3339(self.safe_data["agent-status"]["since"])

@property
def is_subordinate(self):
Expand All @@ -52,7 +52,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 ciso8601.parse_rfc3339(self.safe_data["workload-status"]["since"])

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

import logging

import pyrfc3339
import ciso8601

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 ciso8601.parse_rfc3339(self._user_info.last_connection)

@property
def access(self):
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
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"',
"ciso8601>=2.3.2",
]
[project.optional-dependencies]
dev = [
Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
package_data={"juju": ["py.typed"]},
install_requires=[
"macaroonbakery>=1.1,<2.0",
"pyRFC3339>=1.0,<2.0",
"ciso8601>=2.3.2",
"pyyaml>=5.1.2",
"websockets>=13.0.1",
"paramiko>=2.4.0",
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/test_gocookies.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import unittest
import urllib.request

import pyrfc3339
import ciso8601

from juju.client.gocookies import GoCookieJar

Expand Down Expand Up @@ -223,7 +223,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(ciso8601.parse_rfc3339("2345-11-15T18:16:08Z").timestamp())
self.assertEqual(got_expires, want_expires)

def load_jar(self, content):
Expand Down
Loading