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

graph-builder: add plugin to fetch openshift/cincinnati-graph-data from github #226

Conversation

steveej
Copy link
Contributor

@steveej steveej commented Feb 18, 2020

Add a github_openshift_secondary_metadata_scraper plugin:

This plugin is in preparation for a subsequent plugin which will be able
to parse the fetched data.


@steveej steveej requested a review from wking February 18, 2020 21:02
@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Feb 18, 2020
@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 18, 2020
@steveej steveej force-pushed the pr/plugin-scrape-github-openshift-cincinnati-graph-metadata branch 3 times, most recently from 1986e46 to 609d747 Compare February 20, 2020 08:49
@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Feb 20, 2020
@steveej steveej force-pushed the pr/plugin-scrape-github-openshift-cincinnati-graph-metadata branch from 609d747 to 9b9ac02 Compare February 20, 2020 18:09
@openshift-ci-robot openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. do-not-merge/invalid-owners-file Indicates that a PR should not merge because it has an invalid OWNERS file in it. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 20, 2020
@steveej steveej force-pushed the pr/plugin-scrape-github-openshift-cincinnati-graph-metadata branch 2 times, most recently from 9a8ee92 to f4e4630 Compare February 20, 2020 20:09
@steveej
Copy link
Contributor Author

steveej commented Feb 20, 2020

the OWNERS files of the test fixtures were tripping up prow.

/verify-owners

@openshift-ci-robot
Copy link

The following users are mentioned in OWNERS file(s) but are not members of the openshift org.

Once all users have been added as members of the org, you can trigger verification by writing /verify-owners in a comment.

  • cincinnati-graph-data-approvers
    • graph-builder/src/plugins/openshift_secondary_metadata_parser/test_fixtures/20200220.104838/cincinnati-graph-data/blocked-edges/OWNERS
    • graph-builder/src/plugins/openshift_secondary_metadata_parser/test_fixtures/20200220.104838/cincinnati-graph-data/channels/OWNERS
  • cincinnati-graph-data-reviewers
    • graph-builder/src/plugins/openshift_secondary_metadata_parser/test_fixtures/20200220.104838/cincinnati-graph-data/blocked-edges/OWNERS
    • graph-builder/src/plugins/openshift_secondary_metadata_parser/test_fixtures/20200220.104838/cincinnati-graph-data/channels/OWNERS
  • release-admins
    • graph-builder/src/plugins/openshift_secondary_metadata_parser/test_fixtures/20200220.104838/cincinnati-graph-data/blocked-edges/OWNERS
    • graph-builder/src/plugins/openshift_secondary_metadata_parser/test_fixtures/20200220.104838/cincinnati-graph-data/channels/OWNERS

@steveej steveej force-pushed the pr/plugin-scrape-github-openshift-cincinnati-graph-metadata branch from f4e4630 to ebccd1b Compare February 21, 2020 23:06
@steveej steveej force-pushed the pr/plugin-scrape-github-openshift-cincinnati-graph-metadata branch from ebccd1b to fd2c2c9 Compare February 21, 2020 23:30
@steveej steveej force-pushed the pr/plugin-scrape-github-openshift-cincinnati-graph-metadata branch from fd2c2c9 to 3b57f8a Compare February 24, 2020 19:08
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 24, 2020
@steveej steveej force-pushed the pr/plugin-scrape-github-openshift-cincinnati-graph-metadata branch 2 times, most recently from f57adee to e6110a1 Compare February 24, 2020 19:10
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 24, 2020
@steveej steveej force-pushed the pr/plugin-scrape-github-openshift-cincinnati-graph-metadata branch from e6110a1 to 4153270 Compare February 24, 2020 19:15
@steveej steveej force-pushed the pr/plugin-scrape-github-openshift-cincinnati-graph-metadata branch 4 times, most recently from 2985759 to 2b9fe9d Compare February 24, 2020 21:05
@openshift-ci-robot openshift-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Feb 24, 2020
@steveej steveej changed the title [WIP] Add graph-builder plugins for scraping and parsing openshift/cincinnati-graph-data graph-builder: add github scraper for openshift/cincinnati-graph-data Feb 24, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 24, 2020
@steveej steveej force-pushed the pr/plugin-scrape-github-openshift-cincinnati-graph-metadata branch 2 times, most recently from 827707c to d1bb43f Compare February 24, 2020 21:33
@steveej steveej changed the title graph-builder: add github scraper for openshift/cincinnati-graph-data graph-builder: add plugin to fetch openshift/cincinnati-graph-data from github Feb 24, 2020
@steveej steveej removed the do-not-merge/invalid-owners-file Indicates that a PR should not merge because it has an invalid OWNERS file in it. label Feb 24, 2020
@steveej
Copy link
Contributor Author

steveej commented Feb 25, 2020

/retest

Copy link
Member

@vrutkovs vrutkovs left a comment

Choose a reason for hiding this comment

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

LGTM other than a few typos

@vrutkovs
Copy link
Member

I'm not too happy about binding Cincinnati to Github. Could it be reworked into a generic git repo (use clone instead of fetching data as a tarball)? This would make disconnected installs easier

@steveej steveej force-pushed the pr/plugin-scrape-github-openshift-cincinnati-graph-metadata branch from d1bb43f to 98c3b0f Compare February 25, 2020 12:53
@steveej
Copy link
Contributor Author

steveej commented Feb 25, 2020

I'm not too happy about binding Cincinnati to Github. Could it be reworked into a generic git repo (use clone instead of fetching data as a tarball)? This would make disconnected installs easier

I understand your concern and I can assure you that making GitHub a hard requirement is not a goal.
The next step from here is to finish the graph-builder plugin configuration support.
That will allow us to configure the graph-builder so that the github scraper is disabled, and only the data parser is enabled. The data repository can be transferred and provided to the graph-builder by any means.
This doesn't exclude building additional plugins which download the archive from a fixed URL, shell out to git, use a git library, etc.

@vrutkovs
Copy link
Member

The data repository can be transferred and provided to the graph-builder by any means.

Ah, that makes sense (since data would be basically checked out files if I understood correctly)

@steveej
Copy link
Contributor Author

steveej commented Feb 25, 2020

Ah, that makes sense (since data would be basically checked out files if I understood correctly)

Yes. The plugin in this PR downloads and extracts them, while the plugin in #231 parses the extracted files. I will pick up #74 soon which will implement the missing piece.

Add a github_openshift_secondary_metadata_scraper plugin:
* Implement github v3 API helper module with deserialization tests
* Implement plugin logic
* Test to fetch production data from
  https://github.com/openshift/cincinnati-graph-data

This plugin is in prepration for a subsequent plugin which will be able
to parse the fetched data.
@steveej steveej force-pushed the pr/plugin-scrape-github-openshift-cincinnati-graph-metadata branch from 98c3b0f to 4f0e1ed Compare February 25, 2020 13:57
@steveej
Copy link
Contributor Author

steveej commented Feb 25, 2020

/retest

Copy link
Member

@vrutkovs vrutkovs left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 26, 2020
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: steveeJ, vrutkovs

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot merged commit 2d6f1ca into openshift:master Feb 26, 2020
@steveej steveej deleted the pr/plugin-scrape-github-openshift-cincinnati-graph-metadata branch February 26, 2020 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants