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

Allow passing start and end dates for RW areas sync #177

Merged
merged 8 commits into from
Feb 11, 2025

Conversation

jterry64
Copy link
Member

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Make sure you are requesting to pull a topic/feature/bugfix branch (right side). Don't request your master!
  • Make sure you are making a pull request against the develop branch (left side). Also you should start your branch off our develop.
  • Check the commit's or even all commits' message styles matches our requested structure.
  • Check your code additions will fail neither code linting checks nor unit test.

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Previously, we assumed the /v2/areas/sync API syncs all pending areas to the DB, and then returns all areas still pending. However, it turns out it only returns pending areas in the specified date range. This is max the past 2 days, but we added a change a few months to filter to the past 1 day, since sometimes it was timing out.

This has a few implications:

  • Any time we miss a day due to some syncing error, we actually never analyze those areas again, and they just remain pending forever
  • If syncing fails, we delete the synced areas S3 file to avoid duplicates. However, since we started filtering to one day of pending areas, this isn't an issue anymore. Each day's file won't overlap the last. We may need to adjust to passing specific seconds though, since we're just passing the MMMM-YY-dd, which may have overlap since we're running it about 3 AM UTC.

What is the new behavior?

  • Don't delete areas sync file anymore on failure. We do still need to manually complete failed runs, but don't need to worry about duplicates. Using GTC-3152 to track adding an automated way to handle this.
  • Make sure to dates to the second to /v2/areas/sync to avoid any overlap between calls.
  • To allow us to more easily manually fill in previous days that had failures, allow passing start and end date parameters into the sync function.

Does this introduce a breaking change?

  • Yes
  • No

Other information

Copy link
Contributor

@gtempus gtempus left a comment

Choose a reason for hiding this comment

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

I'm glad this was a small change. Looks good, @jterry64 ! 💯

Copy link
Contributor

@danscales danscales left a comment

Choose a reason for hiding this comment

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

Looks good, but would be good to fix the wrong indent in sync.py.

def __init__(self, sync_types: List[SyncType], sync_version: str = None):
self.sync_version: str = (
def __init__(self, sync_types: List[SyncType], sync_version: str = None, **kwargs):
self.sync_version: str = (
Copy link
Contributor

Choose a reason for hiding this comment

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

This line is indented wrong. Should be four more spaces. I guess maybe Python understands because it's right after the ':" of the function definition, but would be good to fix.

Copy link
Member Author

Choose a reason for hiding this comment

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

It turns out my pre-commit linter wasn't set up properly! Still need to test it all out

@jterry64 jterry64 changed the base branch from master to develop February 11, 2025 17:48
@danscales danscales merged commit e275f46 into develop Feb 11, 2025
3 checks passed
@danscales danscales deleted the hotfix/specify_sync_dates branch February 11, 2025 20:57
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.

3 participants