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 #2481] Add new GitHub export #2539

Merged
merged 22 commits into from
Oct 29, 2024
Merged

Conversation

widal001
Copy link
Collaborator

@widal001 widal001 commented Oct 21, 2024

Summary

Adds new export command for the new GitHub issue structure.

Fixes #2481

Time to review: 10 mins

Changes proposed

What was added, updated, or removed in this PR.

  • Adds GitHubIssues dataset to datasets/issues.py
  • Enables tests that require a slack token to be run with the --slack-token-set pytest flag
  • Creates a new GitHubIssues dataset that stores the content of the new delivery data export
  • Refactors integrations/github.py to make it a sub-package integrations/github/ to store the bash script and graphql queries
  • Adds a new export_delivery_data() to integrations/github/main.py
  • Replaces the old dataset in export_json_to_database() with GitHubIssues

TODO

  • Refactor integrations/github.py to make it a sub-package integrations/github/ to store the bash script and graphql queries
  • Add a new export_delivery_data() to integrations/github/main.py
  • Add a CLI entrypoint to export the data.
  • Replacing the old dataset in export_json_to_database()

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.

  1. Follow the instructions in documentation/analytics/development.md using Docker.
  2. Run make delivery-data-export POINTS_FIELD="Story Points" to export GitHub data using the new format
  3. Run make gh-data-db-import to load the exported data into the Postgres DB
  4. You should see about 1065 rows inserted

Additional information

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

Screenshot 2024-10-28 at 12 21 18 PM

Enables users to run the slack integration tests if they pass the:
`--slack-token-set` flag
This dataset stores a newly transformed version of exported GitHub data.
Adds tests for get_parent_with_type()
Enables us to store multiple scripts in the integrations/github/ dir
- export_sprint_data()
- export_roadmap_data()
Catches and logs validation errors instead of panicking
during `populate_issue_lookup()`
Replaces numpy.nan with None in BaseDataset.to_dict() method
Exports sprint and roadmap data using the new structure
@github-actions github-actions bot added the shell label Oct 24, 2024
@acouch
Copy link
Collaborator

acouch commented Oct 25, 2024

Approach looks great so far 🚀

@github-actions github-actions bot added the ci/cd label Oct 25, 2024
Loads a dataset directly from a JSON file with the same shape.
Loads the `GitHubIssues` dataset instead of `SprintBoard` into the db.
The `SprintBoard` represents the old method for exporting gh data,
and `GitHubIssues` is the new way GitHub data is represented
@widal001 widal001 marked this pull request as ready for review October 28, 2024 16:22
@coilysiren
Copy link
Collaborator

👀

@@ -163,6 +164,17 @@ roadmap-data-export:
--project $(ROADMAP_PROJECT) \
--output-file $(ROADMAP_FILE)

delivery-data-export:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want this running in AWS?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not yet! I'll think want to finish some subset of the following tickets:

Then I'll create a separate ticket for which commands to run in AWS and tag you on it @coilysiren

coilysiren
coilysiren previously approved these changes Oct 28, 2024
@acouch
Copy link
Collaborator

acouch commented Oct 28, 2024

Looks great once we are green. Noting the documentation isn't updated but that could wait for a follow-up. There is this analytics doc ticket that could be used #2130 or another created.

Copy link
Collaborator

@DavidDudas-Intuitial DavidDudas-Intuitial left a comment

Choose a reason for hiding this comment

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

LGTM

The extra spaces in the SPRINT_FIELD and POINTS_FIELD before the comment
were being passed through to the CLI
@widal001
Copy link
Collaborator Author

@coilysiren and @DavidDudas-Intuitial I fixed one bug with the Makefile (fix(analytics): Extra space in Makefile variable) that I didn't catch until I was working on the sprint burndown PR, would you all mind reviewing again?

@widal001 widal001 merged commit dc241a4 into main Oct 29, 2024
7 checks passed
@widal001 widal001 deleted the issue-2481-add-new-github-export branch October 29, 2024 16:48
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.

Add new GitHub export script to /analytics codebase
4 participants