Skip to content
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

Jump to bottom

Delegate date strings formatting to datetime.isoformat() #6413

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from
Draft

Delegate date strings formatting to datetime.isoformat() #6413

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
4 changes: 1 addition & 3 deletions securedrop/journalist_app/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@
from werkzeug.exceptions import HTTPException # noqa: F401

_insecure_views = ['main.login', 'static']
# Timezone-naive datetime format expected by SecureDrop Client
API_DATETIME_FORMAT = "%Y-%m-%dT%H:%M:%S.%fZ"


def get_logo_url(app: Flask) -> str:
Expand Down Expand Up @@ -75,7 +73,7 @@ class JSONEncoder(json.JSONEncoder):

def default(self, obj: 'Any') -> 'Any':
if isinstance(obj, datetime):
return obj.strftime(API_DATETIME_FORMAT)
return obj.isoformat()
super(JSONEncoder, self).default(obj)

app.json_encoder = JSONEncoder
Expand Down
2 changes: 1 addition & 1 deletion securedrop/tests/test_journalist_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@

def assert_valid_timestamp(timestamp: str) -> None:
"""verify the timestamp is encoded in the format we want"""
dt_format = "%Y-%m-%dT%H:%M:%S.%fZ"
dt_format = "%Y-%m-%dT%H:%M:%S.%f+00:00"
Copy link
Member

Choose a reason for hiding this comment

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

Can we use datetime.fromisoformat() to match securedrop-sdk? Looking at https://github.com/freedomofpress/securedrop-sdk/blob/31ea11e3f9d8c243c0fbe2a19bbdd4f75bd32bd0/sdclientapi/timestamps.py

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we can! I left this one as-is because I like having distinct implementations for the tests and the code under test. I found that using the format string made particularly clear the expectation in the test file. But all that being said, I'd say as a securedrop repo maintainer it's your call, and I don't personally have strong feelings about it.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe both then? I would like to make sure we're at least testing with what the sdk is actually using, which is now fromisoformat().

assert timestamp == datetime.strptime(timestamp, dt_format).strftime(dt_format)


Expand Down