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.

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

Add Timezone Data to DB Backend #232

Merged
merged 3 commits into from
May 11, 2021

Conversation

MRichards99
Copy link
Collaborator

This PR will close #225

Description

This change helps to make the DB backend become more aligned with the ICAT backend by having timezones when displaying datetimes on the API.

It seems the DB backend doesn't actually use timezones anywhere - I tried setting the timezone flag as pointed out in the docs but that didn't have any impact. Instead, I've made the DB backend display the local timezone for the API. This seems to be the best compromise I could find to make the two backends aligned.

The DB backend now uses the DateHandler so will have the same format for datetimes as the ICAT backend.

I also noticed that the get session details endpoint doesn't use timezones on the ICAT backend. I fixed that on 9b2dc7d.

Testing Instructions

Check that both backends have display datetime in the same format (including the timezone part of the output)

  • Review code
  • Check GitHub Actions build
  • Review changes to test coverage

Agile Board Tracking

Connect to #225

@codecov
Copy link

codecov bot commented May 5, 2021

Codecov Report

Merging #232 (9b2dc7d) into master (0c68549) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #232      +/-   ##
==========================================
+ Coverage   89.45%   89.48%   +0.02%     
==========================================
  Files          31       31              
  Lines        2296     2301       +5     
  Branches      191      191              
==========================================
+ Hits         2054     2059       +5     
  Misses        211      211              
  Partials       31       31              
Impacted Files Coverage Δ
datagateway_api/common/constants.py 100.00% <100.00%> (ø)
datagateway_api/common/database/models.py 96.43% <100.00%> (+0.01%) ⬆️
datagateway_api/common/icat/helpers.py 92.76% <100.00%> (+0.03%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0c68549...9b2dc7d. Read the comment docs.

Copy link

@sam-glendenning sam-glendenning left a comment

Choose a reason for hiding this comment

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

Works fine. Example output with a GET request to /sessions:

{
    "id": "e5525746-b235-11eb-bbcb-000c2906e44d",
    "expireDateTime": "2021-05-12 01:50:11-07:00",
    "username": "simple/root"
}

@MRichards99 MRichards99 merged commit 827454b into master May 11, 2021
@MRichards99 MRichards99 deleted the refactor/datehandler-db-backend-#225 branch May 11, 2021 09:26
Sign up for free to 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.

Use DateHandler when dealing with dates on DB backend
2 participants