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

[Issue #2548] Swap analytics to new step function pattern #2546

Merged
merged 3 commits into from
Oct 30, 2024

Conversation

coilysiren
Copy link
Collaborator

@coilysiren coilysiren commented Oct 23, 2024

Summary

Relates to #2548

Time to review: 2 mins

Changes proposed

Swaps the analytics application to the new step functions pattern that I originally merged in #2506. The size of the red diff and the simplicity of the new configuration show have valuable the new pattern is.

Context for reviewers

The step function is now running, but the CLI itself is currently broken. eg. the CLI is currently broken everywhere, even locally. Fixing that is someone else's job though!

@coilysiren coilysiren changed the title swap analytics to new sfn pattern [Issue #2548] Swap analytics to new step function pattern Oct 23, 2024
@coilysiren coilysiren marked this pull request as ready for review October 23, 2024 14:45
@coilysiren coilysiren merged commit cd1844c into main Oct 30, 2024
7 checks passed
@coilysiren coilysiren deleted the kai/analytics-sfn branch October 30, 2024 14:08
DavidDudas-Intuitial added a commit that referenced this pull request Nov 7, 2024
## Summary
Fixes #2665 

### Time to review: __1 min__

## Changes proposed
> What was added, updated, or removed in this PR.

Added `gh-transform-and-load` command to existing `make gh-data-export`
command. I'm not sure if this is sufficient or correct, but I'm taking a
guess based on what I see in
#2546 and
#2506.

## Context for reviewers
> Testing instructions, background context, more in-depth details of the
implementation, and anything else you'd like to call out or ask
reviewers. Explain how the changes were verified.

In the analytics work stream, we have a new CLI command `make
gh-transform-and-load` for transforming and loading (some) GitHub data.
Per issue #2665, that command should be run daily, after the existing
`gh-data-export` command which exports data from Github.

I see that `scheduled_jobs.tf` seems to be the mechanism by which `make
gh-data-export` runs daily. In this PR I'm taking and educated guess and
attempting to add `gh-transform-and-load` to the existing job, and
requesting feedback from @coilysiren as to whether this is the correct
approach.

## Additional information
> Screenshots, GIF demos, code examples or output to help show the
changes working as expected.

Co-authored-by: kai [they] <[email protected]>
babebe pushed a commit that referenced this pull request Nov 7, 2024
## Summary
Fixes #2665 

### Time to review: __1 min__

## Changes proposed
> What was added, updated, or removed in this PR.

Added `gh-transform-and-load` command to existing `make gh-data-export`
command. I'm not sure if this is sufficient or correct, but I'm taking a
guess based on what I see in
#2546 and
#2506.

## Context for reviewers
> Testing instructions, background context, more in-depth details of the
implementation, and anything else you'd like to call out or ask
reviewers. Explain how the changes were verified.

In the analytics work stream, we have a new CLI command `make
gh-transform-and-load` for transforming and loading (some) GitHub data.
Per issue #2665, that command should be run daily, after the existing
`gh-data-export` command which exports data from Github.

I see that `scheduled_jobs.tf` seems to be the mechanism by which `make
gh-data-export` runs daily. In this PR I'm taking and educated guess and
attempting to add `gh-transform-and-load` to the existing job, and
requesting feedback from @coilysiren as to whether this is the correct
approach.

## Additional information
> Screenshots, GIF demos, code examples or output to help show the
changes working as expected.

Co-authored-by: kai [they] <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants