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

Generate Cred Nightly with GitHub Actions #17

Closed
wants to merge 71 commits into from
Closed

Generate Cred Nightly with GitHub Actions #17

wants to merge 71 commits into from

Conversation

vsoch
Copy link

@vsoch vsoch commented Feb 13, 2020

This pull request will add a recipe that uses vsoch/cred-action to generate cred on a nightly basis. Note that the file we are adding here is a GitHub workflow, and can be customized per the example here.

Why a GitHub Action?

This is better explained in the README of the cred action repository, but basically a GitHub action can offer a consistent interface via the workflow parameters that will allow for future changes to the underlying stuff that gets run in the sourcecred container. In simpler terms, we can change our mind about the sourcecred client and keep the same / similar client facing interface.

Why Containers?

If we have the action be based on the latest (dev or release) of sourcecred, which is already built in an automated way, this makes our life easy - we don't have to update the cred instance when sourcecred is updated in terms of building a new container.

I'd like to suggest the following to move forward:

  • move the cred-action to be under the sourcecred organization - this also means update the example workflows (here and in the examples folder) to use it.
  • once moved, get @Beanow feedback on how to re-add the cache (I am starting with a stripped, simple implementation that uses containers and isn't taking advantage of the cache... yet!)
  • I noticed that the project is loaded twice and this doubles the length of the build. This is noted to be a hack and I'd like to discuss how this can be fixed - the comment there suggests that the instance system can fix this (so I'd like to talk about how!)
  • I'd like to add jq to the base container so it doesn't need to be installed with the action container (adds a little extra build / update time for the container).

Questions that I have

  1. What other parameters should / need to be exposed?
  2. Is the frequency (nightly late) a reasonable run time?
  3. Is an assumption that users would want to deploy to GitHub pages via the docs folder reasonable?

For the last point, I give the user the option to set automated to true (meaning it pushes to a branch directly) or leave the default false, meaning it opens a PR as it did in my testing case. Is there a third likely option - that the user doesn't want to open a PR OR an automated deploy? In this case, I'd suggest that I add a parameter, something like skip_deploy to just not do anything after (and let the user workflow take over). I think in this case we'd need to define an output to the workflow to ensure that files are available after, although I'm not sure.

Links that might be useful

Looking forward to getting feedback!

teamdandelion and others added 30 commits August 16, 2019 21:53
Summary:
Following the newly updated instructions in the README.

Test Plan:
Running `python3 -m http.server` at the root of this repository and then
following the test plan in sourcecred/sourcecred#1305 confirms that the
fix for sourcecred/sourcecred#1304 is correctly integrated.
I've taken out the other projects (ipld, libp2p, filecoin-project) and
am just focusing on SourceCred's own cred for now. There are three
subprojects:
- SourceCred github
- SourceCred discourse
- SourceCred combined

I've added an updater script and a bit more info in the README.

Test plan: Local inspection via running a simple http server.
Almost the same as before, but I renamed the combined project "@sourcecred"
so as to not break links
Mostly trying to move some cred away from GitHub comment based activity,
for reasons discussed in [discourse][1], and reduce the tendency of
highly-commented-upon PRs to skyrocket in score.

I also moved the default user weights to 0, since they don't actually do
anything.

Test plan: Inspected the cred, it seems "ok".

[1]: https://discourse.sourcecred.io/t/preliminary-credsperiment-cred/219/4
A bit redundant as there's no corresponding Discourse account, but I'm
thinking for payouts I will filter to only compute for SourceCred
identities, so this is convenient.
It's kind of hacky, but should work.
Also update docs and save those scores.
Remove identity resolution for vsoch, no longer needed.
Scores are as computed [in the Observable notebook][1]. The score JSON
was downloaded using the "Save Distribution History" button just above
the "Full Data" sheet.

Test plan: Verify that the hash of this file matches the expected hash
in the notebook:

```
$ sha256sum distributionHistory.json
3f75966fdf0e01737952573ef290cebb22298e026ff9697d26e29c20e7bcfd8e
```

[1]: https://observablehq.com/@decentralion/credsperiment-week-1
Generated by running `./update.sh`.
Scores are as computed [in the Observable notebook][1]. The score JSON
was downloaded using the "Save Distribution History" button just above
the "Full Data" sheet.

Test plan: Verify that the hash of this file matches the expected hash
in the notebook:

```
$ sha256sum distributionHistory.json
bfee2286b361380d01099a01c42c333c302a30a0e0903cdebe368b4c8b53f036
```

[1]: https://observablehq.com/@decentralion/sourcecred-credsperiment-week-2
* Include payouts repo in github.json

* Include payouts repo in combined.json
Scores are as computed [in the Observable notebook][1]. The score JSON
was downloaded using the "Save Distribution History" button just above
the "Full Data" sheet.

Test plan: Verify that the hash of this file matches the expected hash
in the notebook:

```
$ sha256sum distributionHistory.json
87bb124576b9a4554e07c0ec8da7e92c7230723ada9f96cc29b9ae8f27c3ee19
```

[1]: https://observablehq.com/@decentralion/sourcecred-credsperiment-week-3
Matches expected hash:
9c3c8acf17a1e8dc0e0c652a6a2f1a884ec5ea827ad43205dcf3fb5415b960bf
I defined a simple format for the transfers, which just gives the
basic necessary info (who, how much, when) along with arbitrary
references. I linked to the open collective payout and the discourse
payout request in the references.

No code yet consumes this information.
The work that goes into this repository should count too. :)

Test plan: Run the update script, verify that it loads without issue.
This is a small experiment in having something like [Cred Bounties][1]
before we've actually implemented the [Initiatives Plugin][2].

Basically, we're giving a higher weight to a few initiatives:
- Produce the SourceCred Podcast: Shipped the podcast!
- Discourse Reference Detection: Recently completed.
- Champions & Heroes: An exciting contribution. :)
- Initiatives Plugin: Nice progress so far.

The weight choices are moderate, so it doesn't have a _huge_ effect on
cred--we'll hold off on big effects for after the initiatives plugin and
cred bounties both land. But this is a nice way to recognize some
exciting progress so far.

[1]: https://discourse.sourcecred.io/t/enable-cred-bounties/257
[2]: https://discourse.sourcecred.io/t/write-the-initiatives-plugin/269
As calculated [here]. Shasum matches:

```
❯ sha256sum distributionHistory.json
5a0e5c16e67f53f82981f304a4ad85e2437b08491b88c5cd010575a516d6f5da
```

[here]: https://observablehq.com/@decentralion/credsperiment-week-5
As computed here:
https://observablehq.com/@decentralion/credsperiment-week-6

Expected distributionHistory hash:
9186964bd1fb233e7c929ad7b397201940b5a1a434a0075c14f0bf8d2f5b49fc
* Record a transfer from s-ben to protocol

Needs review/ack by @s_ben

* Update references for the transfer
teamdandelion and others added 15 commits January 14, 2020 10:35
This update is needed as of sourcecred/sourcecred#1558. I didn't make
any changes to the weights themselves. As discussed in the review there,
the weights semantics changed in some edge cases; happily, none of those
edge cases were present in our weights.

Test plan: I've re-computed cred using the new weights (built against
sourcecred master) and verified that the weights are set correctly in
the UI.
I accidentally created a series of "spam" issues in
sourcecred/sourcecred which had no content. (sourcecred/sourcecred#1544
through sourcecred/sourcecred#1556) This commit sets the weight to those
issues to zero, so that I do not receive any cred for them.

Test plan: Inspect this change carefully.
Generated via ./update.sh
This changes the weights so that on Discourse, we only mint cred based
on likes, rather than to raw activity. I've set the like-weight to 16,
which is the same as a GitHub pull request. This is a somewhat arbitrary
decision and likely to be re-evaluated in the future.

See extensive discussion around this change at [this Discourse post][1].

Test plan: Review, and I'll compile a full score update as a followon.

[1]: https://discourse.sourcecred.io/t/minting-discourse-cred-on-likes-not-posts/603
Signed-off-by: vsoch <vsochat@stanford.edu>
Signed-off-by: vsoch <vsochat@stanford.edu>
Signed-off-by: vsoch <vsochat@stanford.edu>
Copy link
Contributor

@wchargin wchargin left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request! Let’s defer actually merging this until
vsoch/cred-action has itself been reviewed (at this point it might be
easier for me to just send pull requests, since the code has already
merged into that repo).

.github/workflows/generate-cred.yml Outdated Show resolved Hide resolved
.github/workflows/generate-cred.yml Outdated Show resolved Hide resolved
@vsoch
Copy link
Author

vsoch commented Feb 14, 2020

hey @wchargin ! Sorry reading this after the other PR to add jq - do you want to open a PR to cred-action with the changes you are suggesting? That might be easier since you've used python with the json tool previously.

Update: I tested, verified working with python, and merged. The PR here has been updated.

@wchargin
Copy link
Contributor

Okay, I’ll send PRs for those issues that I can just clean up; others
require questions, so I’ll post reviews on the pull requests that added
the code in question. Could you please grant me push access to
vsoch/cred-action so that I can create dependent pull requests?
(I don’t need merge-to-master permissions.)

@vsoch
Copy link
Author

vsoch commented Feb 15, 2020

Sure thing! There isn't that level of granularity for permissions so you have write, but thank you for that courtesy! And if there is granularity somewhere that I don't know about... tell me about that too :)
I'll be back at keyboard this afternoon... still ransacking stores for discount Valentine's candy, lol :P

@vsoch
Copy link
Author

vsoch commented Feb 15, 2020

Here is the list of general TODOs I was sharing with @decentralion:

  • add cache back in to speed it up (look in to both the sourcecred cache and for the base container, last I checked the docker cache wasn't ready yet officially but there are non-GitHub developer options (but I don't recommend using)
  • see if we can eliminate the need to load twice (this might depend on working on sourcecred under the hood and take more time
  • look critically at action.yml and see if there are other options (called inputs) desired or envars.
  • Then when it’s mostly good, we can move it to the sourcecred org
  • Update docs and example recipes for new location
  • Do a tag / release so the workflow we share can have a proper tag and not an ugly commit.
  • Put on GitHub marketplace if desired! (the action.yml is present)

@wchargin
Copy link
Contributor

And if there is granularity somewhere that I don't know about... tell
me about that too :)

You can create a branch protection rule for master and set “Restrict
who can push to matching branches”. (This is the configuration that we
use for sourcecred/sourcecred.)

What is the best way to test my changes before requesting your review?

@vsoch
Copy link
Author

vsoch commented Feb 15, 2020

Oh yes, I remember that now!

For testing, what I did is fork the cred repo and then add a GitHub workflow there, and you can add to trigger on PR and then open it once. You’ll only need to do one commit because you can re-run the workflow with a changed action to test again. Does that work?

@vsoch
Copy link
Author

vsoch commented Feb 17, 2020

Update for work going on here:

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.

None yet

4 participants