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

Fix the timezone issues when pulling records from file db. #829

Merged
merged 2 commits into from
May 8, 2019

Conversation

wtgee
Copy link
Member

@wtgee wtgee commented May 5, 2019

Description

Quick fix to the timezone issues discussed in #821. This can close #821 for now but as mentioned in the Issue there might be a better solution on the insertion of the dates rather than the extraction.

Related Issue

#821

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

@wtgee wtgee added bug Quick PR This PR should be quick to review. Merge on Approval labels May 5, 2019
@wtgee wtgee requested review from jamessynge and AnthonyHorton May 5, 2019 23:31
@codecov
Copy link

codecov bot commented May 5, 2019

Codecov Report

Merging #829 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #829   +/-   ##
========================================
  Coverage    81.25%   81.25%           
========================================
  Files           68       68           
  Lines         5516     5516           
  Branches       759      759           
========================================
  Hits          4482     4482           
  Misses         837      837           
  Partials       197      197

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 a491093...96a5626. Read the comment docs.

Copy link
Collaborator

@AnthonyHorton AnthonyHorton left a comment

Choose a reason for hiding this comment

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

This is probably OK as a quick fix for the problem at hand, but as I note in #821 (comment) the .replace(tzinfo=None) is stripping the timezone info without first converting to UTC. If timestamps in timezones other than UTC ever ended up in the DB then this would cause errors.

bin/peas_shell Outdated Show resolved Hide resolved
@wtgee wtgee merged commit eb2eb15 into panoptes:develop May 8, 2019
@wtgee wtgee deleted the fix-peas-time branch May 8, 2019 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Merge on Approval Quick PR This PR should be quick to review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Metadata DB date fields timezone - BSON datetime
2 participants