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

🌱 cron: generalize and expose worker (6/n) #2317

Merged
merged 19 commits into from
Oct 19, 2022

Conversation

spencerschrock
Copy link
Member

What kind of change does this PR introduce?

Part of a series of refactors to generalize the cron infrastructure to enable re-use by the ossf criticality score project

What is the current behavior?

The worker logic is scorecard specific and can't be re-used.

What is the new behavior (if this is a feature change)?**

The general logic has been separated from the scorecard specific logic.
Criticality should be able to import the required modules and create their own worker which implements the interface.

The following packages are (re) exposed in cron:

  • config
  • data
  • worker
  • Tests for the changes have been added (for bug fixes/features)

more of a refactor of existing code. the worker release test should catch any major errors.

Which issue(s) this PR fixes

NONE

Special notes for your reviewer

Does this PR introduce a user-facing change?

For user-facing changes, please add a concise, human-readable release note to
the release-note

(In particular, describe what changes users might need to make in their
application as a result of this pull request.)

NONE

Signed-off-by: Spencer Schrock <[email protected]>
Signed-off-by: Spencer Schrock <[email protected]>
Signed-off-by: Spencer Schrock <[email protected]>
Signed-off-by: Spencer Schrock <[email protected]>
Signed-off-by: Spencer Schrock <[email protected]>
Signed-off-by: Spencer Schrock <[email protected]>
@spencerschrock
Copy link
Member Author

Not sure if Friday is the best time to merge something like this, but wanted to get it up for discussion for any changes.
@oliverchang @calebbrown fyi

@codecov
Copy link

codecov bot commented Sep 30, 2022

Codecov Report

Merging #2317 (b1b9601) into main (900701c) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2317   +/-   ##
=======================================
  Coverage   40.61%   40.61%           
=======================================
  Files         112      112           
  Lines        8831     8831           
=======================================
  Hits         3587     3587           
  Misses       4984     4984           
  Partials      260      260           

@github-actions
Copy link

@spencerschrock spencerschrock temporarily deployed to integration-test October 4, 2022 17:12 Inactive
@spencerschrock
Copy link
Member Author

876acb0 will cause metadata to be populated to the BigQuery results. Might be worth spinning this off in it's own PR

@github-actions
Copy link

github-actions bot commented Oct 4, 2022

@github-actions
Copy link

Stale pull request message

cron/worker/worker.go Show resolved Hide resolved
Signed-off-by: Spencer Schrock <[email protected]>
@spencerschrock spencerschrock temporarily deployed to integration-test October 19, 2022 17:00 Inactive
@github-actions
Copy link

@azeemshaikh38
Copy link
Contributor

876acb0 will cause metadata to be populated to the BigQuery results. Might be worth spinning this off in it's own PR

Oh, so this was the bug all along. Yeah doing this in a separate PR SGTM.

This reverts commit 876acb0.

Will send separate PR.

Signed-off-by: Spencer Schrock <[email protected]>
@spencerschrock spencerschrock temporarily deployed to integration-test October 19, 2022 18:06 Inactive
@github-actions
Copy link

@azeemshaikh38 azeemshaikh38 enabled auto-merge (squash) October 19, 2022 18:29
Signed-off-by: Spencer Schrock <[email protected]>
auto-merge was automatically disabled October 19, 2022 20:39

Head branch was pushed to by a user without write access

@spencerschrock spencerschrock temporarily deployed to integration-test October 19, 2022 20:39 Inactive
@azeemshaikh38 azeemshaikh38 enabled auto-merge (squash) October 19, 2022 20:50
@github-actions
Copy link

@azeemshaikh38 azeemshaikh38 merged commit f979097 into ossf:main Oct 19, 2022
latortuga71 pushed a commit to latortuga71/scorecard that referenced this pull request Oct 27, 2022
* WIP

Signed-off-by: Spencer Schrock <[email protected]>

* Appease linter.

Signed-off-by: Spencer Schrock <[email protected]>

* Update Makefile for worker

Signed-off-by: Spencer Schrock <[email protected]>

* Extract already completed request sanity check.

Signed-off-by: Spencer Schrock <[email protected]>

* Add worker test.

Signed-off-by: Spencer Schrock <[email protected]>

* remove logger from worker interface

Signed-off-by: Spencer Schrock <[email protected]>

* move cron data and worker out of cron/internal

Signed-off-by: Spencer Schrock <[email protected]>

* Move config out of internal.

Signed-off-by: Spencer Schrock <[email protected]>

* Document worker interface.

Signed-off-by: Spencer Schrock <[email protected]>

* Fix typo which prevented metadata from going to cron job.

Signed-off-by: Spencer Schrock <[email protected]>

* Address feedback.

Signed-off-by: Spencer Schrock <[email protected]>

* Revert "Fix typo which prevented metadata from going to cron job."

This reverts commit 876acb0.

Will send separate PR.

Signed-off-by: Spencer Schrock <[email protected]>

* Fix linter.

Signed-off-by: Spencer Schrock <[email protected]>

Signed-off-by: Spencer Schrock <[email protected]>
Signed-off-by: latortuga <latortugaaaa>
N8BWert pushed a commit to N8BWert/scorecard that referenced this pull request Nov 28, 2022
* WIP

Signed-off-by: Spencer Schrock <[email protected]>

* Appease linter.

Signed-off-by: Spencer Schrock <[email protected]>

* Update Makefile for worker

Signed-off-by: Spencer Schrock <[email protected]>

* Extract already completed request sanity check.

Signed-off-by: Spencer Schrock <[email protected]>

* Add worker test.

Signed-off-by: Spencer Schrock <[email protected]>

* remove logger from worker interface

Signed-off-by: Spencer Schrock <[email protected]>

* move cron data and worker out of cron/internal

Signed-off-by: Spencer Schrock <[email protected]>

* Move config out of internal.

Signed-off-by: Spencer Schrock <[email protected]>

* Document worker interface.

Signed-off-by: Spencer Schrock <[email protected]>

* Fix typo which prevented metadata from going to cron job.

Signed-off-by: Spencer Schrock <[email protected]>

* Address feedback.

Signed-off-by: Spencer Schrock <[email protected]>

* Revert "Fix typo which prevented metadata from going to cron job."

This reverts commit 876acb0.

Will send separate PR.

Signed-off-by: Spencer Schrock <[email protected]>

* Fix linter.

Signed-off-by: Spencer Schrock <[email protected]>

Signed-off-by: Spencer Schrock <[email protected]>
Signed-off-by: nathaniel.wert <[email protected]>
N8BWert pushed a commit to N8BWert/scorecard that referenced this pull request Nov 28, 2022
* WIP

Signed-off-by: Spencer Schrock <[email protected]>

* Appease linter.

Signed-off-by: Spencer Schrock <[email protected]>

* Update Makefile for worker

Signed-off-by: Spencer Schrock <[email protected]>

* Extract already completed request sanity check.

Signed-off-by: Spencer Schrock <[email protected]>

* Add worker test.

Signed-off-by: Spencer Schrock <[email protected]>

* remove logger from worker interface

Signed-off-by: Spencer Schrock <[email protected]>

* move cron data and worker out of cron/internal

Signed-off-by: Spencer Schrock <[email protected]>

* Move config out of internal.

Signed-off-by: Spencer Schrock <[email protected]>

* Document worker interface.

Signed-off-by: Spencer Schrock <[email protected]>

* Fix typo which prevented metadata from going to cron job.

Signed-off-by: Spencer Schrock <[email protected]>

* Address feedback.

Signed-off-by: Spencer Schrock <[email protected]>

* Revert "Fix typo which prevented metadata from going to cron job."

This reverts commit 876acb0.

Will send separate PR.

Signed-off-by: Spencer Schrock <[email protected]>

* Fix linter.

Signed-off-by: Spencer Schrock <[email protected]>

Signed-off-by: Spencer Schrock <[email protected]>
Signed-off-by: nathaniel.wert <[email protected]>
raghavkaul pushed a commit to raghavkaul/scorecard that referenced this pull request Feb 9, 2023
* WIP

Signed-off-by: Spencer Schrock <[email protected]>

* Appease linter.

Signed-off-by: Spencer Schrock <[email protected]>

* Update Makefile for worker

Signed-off-by: Spencer Schrock <[email protected]>

* Extract already completed request sanity check.

Signed-off-by: Spencer Schrock <[email protected]>

* Add worker test.

Signed-off-by: Spencer Schrock <[email protected]>

* remove logger from worker interface

Signed-off-by: Spencer Schrock <[email protected]>

* move cron data and worker out of cron/internal

Signed-off-by: Spencer Schrock <[email protected]>

* Move config out of internal.

Signed-off-by: Spencer Schrock <[email protected]>

* Document worker interface.

Signed-off-by: Spencer Schrock <[email protected]>

* Fix typo which prevented metadata from going to cron job.

Signed-off-by: Spencer Schrock <[email protected]>

* Address feedback.

Signed-off-by: Spencer Schrock <[email protected]>

* Revert "Fix typo which prevented metadata from going to cron job."

This reverts commit 876acb0.

Will send separate PR.

Signed-off-by: Spencer Schrock <[email protected]>

* Fix linter.

Signed-off-by: Spencer Schrock <[email protected]>

Signed-off-by: Spencer Schrock <[email protected]>
@spencerschrock spencerschrock deleted the cron-worker-refactor branch May 24, 2023 17:02
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.

2 participants