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

Resolves ENGA-932 "Modify BigQueryExporter class export() method to accept a pull_date" #5

Merged
merged 8 commits into from
Nov 28, 2023

Conversation

DevDaveFrame
Copy link
Collaborator

@DevDaveFrame DevDaveFrame commented Nov 28, 2023

ENGA-932

Non-technical Context

For the sake of backfilling or coordinating a pull date across multiple models, we want the export function to accept a pull_date argument

Technical Context

I added the pull_date parameter which defaults to None. The pull_time used internally by the method is then set to either the specified pull_date, or now if no pull_date is set.

As an aside, I added .order_by('id') to the default queryset method to accommodate the batching of querysets where ordering avoids reprocessing or missing records.

Developer

  • Ensure code complies with the style guide
  • Devise edge cases to test the bug fix or feature being merged
  • Make sure the code is clean and understandable
  • Ensure code is not overly inefficient
  • Code changes include a migration
    • Migration modifies large table and has been run against full db to estimate completion time
    • Migration modifies a small table and has only been run against slim db
    • There are no conflicting migration leaf nodes
  • Ensure documentation is understandable and accurate, including:
    • Code comments and doc strings
    • Technical documentation

Reviewer

  • Ensure code complies with the style guide
    • If there are few or minor issues, add inline comments
    • If there are many issues or it seems like the developer did not follow the style guide, leave a comment asking them to do so
  • Devise edge cases to test the bug fix or feature being merged
  • Make sure the code is clean and understandable
    • Ask for inline comments on unclear sections
  • Ensure code is not overly inefficient
  • Ensure documentation is understandable and accurate, including:
    • Code comments and doc strings
    • Technical documentation
  • There are no conflicting migration leaf nodes

bigquery_exporter/base.py Outdated Show resolved Hide resolved
DevDaveFrame and others added 2 commits November 28, 2023 15:33
Co-authored-by: Eric Chan <github@ericchan.info>
Copy link
Contributor

@echan217 echan217 left a comment

Choose a reason for hiding this comment

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

LGTM!

@DevDaveFrame DevDaveFrame merged commit 3b6411f into main Nov 28, 2023
1 check passed
@DevDaveFrame DevDaveFrame deleted the ENGA-932-export-method-pull-date-param branch November 28, 2023 23:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants