Skip to content

add wclouser_fxa_db_counts job. Fixes DSRE-1779 #295

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

clouserw
Copy link
Member

@clouserw clouserw commented Nov 1, 2024

Checklist for reviewer:

  • Commits should reference a bug or github issue, if relevant (if a bug is
    referenced, the pull request should include the bug number in the title)
  • Scan the PR and verify that no changes (particularly to
    .circleci/config.yml) will cause environment variables (particularly
    credentials) to be exposed in test logs
  • Ensure the container image will be using permissions granted to
    telemetry-airflow
    responsibly.

@clouserw clouserw force-pushed the wclouser-fxa-db-counts branch from d533d5c to cba4b95 Compare November 12, 2024 23:18
@clouserw
Copy link
Member Author

@akkomar for review please

Copy link
Contributor

@akkomar akkomar left a comment

Choose a reason for hiding this comment

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

There is a test failure that might have something to do with pytest setup. Since we don't really have meaningful tests here, I'd remove the skeleton files (suggested this inline).

More comments/suggestions inline.

]

# Insert rows into the new table
errors = client.insert_rows_json("mozdata.analysis.wclouser_fxa_health_db_counts", rows_to_insert)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is fine in an MVP, but we can also parameterize this to put this data in accounts_backend_derived.
Note that tables created in analysis dataset now have default expiration of 6 months which can be changed manually.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the note on expiration. I'll update that on my table. I don't have strong feelings on where this goes, fwiw.

@clouserw clouserw force-pushed the wclouser-fxa-db-counts branch from cba4b95 to 857818c Compare November 14, 2024 15:35


@click.command()
@click.option("--bq_project_id", help="GCP BigQuery project id", show_default=True, default="moz-fx-fxa-prod")
Copy link
Member

Choose a reason for hiding this comment

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

Recognizing this is probably going to be replaced by something in bqetl: if this was planned to be running via airflow, moz-fx-data-shared-prod is a more appropriate project id to default to since airflow doesn't have roles/bigquery.jobUser access to the fxa projects.

@akkomar
Copy link
Contributor

akkomar commented Nov 15, 2024

We'll move this to bigquery-etl to make this simpler. I filed a draft PR: mozilla/bigquery-etl#6511 that I'll work on.

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