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

Generators workflow #968

Merged
merged 5 commits into from
Apr 22, 2019
Merged

Generators workflow #968

merged 5 commits into from
Apr 22, 2019

Conversation

protolambda
Copy link
Collaborator

After experimenting heavily with CI and generators, I decided it's better to keep generator triggering outside of CI for now.
What I experimented with:

  • Mirror of eth2.0-specs, configured with circle CI
  • Output repository with Git LFS
  • Configured SSH key secret with Circle CI, and pubkey known with output repo to allow for write access.
  • Cached generator venvs
  • Shallow clone output repo
  • trigger builds based on branch name prefixes, output to outputs repo on that same branch name, for parallel/async/fixing scenarios

Reasons to not approach this further as of now:

  • Unreasonable complexity for something relatively formal and low-frequency
  • Circle CI job configuration is poor. No way to configure paralllel jobs succinctly: repeats image, working dir, workflow dependency, and branch presets for every generator. I would rather keep it like the make-file, just automatic detection.
    • Note: we could put them all in one job, and allocate more CPU resources.
  • Poor Git LFS support with circle CI. Not sure if I'm missing a command, but having the LFS attributes there, the new commits made by the bot did not add the files as LFS files properly. Likely have to install LFS, and activate it before making the commit.
  • Not happy with the branch workflow:
    • Triggers based on a merge into a specific branch seem nice, but I dislike the noise it creates.
    • Anyone can create such a branch and make the CI run. The SSH key is not passed to runs triggered by outside contributors, but still feel like the build itself shouldn't trigger so easily.

Proposal to move forward:

  • Run generators locally. No explicit caching necessary, the venvs are all right there (unless you make clean)
  • Make generators run in parallel, make -j 4 gen_yaml_tests to run with 4 cores. Fixes to make this work + documentation included in this PR.
  • Output tests to <repo root>/eth2.0-spec-tests/tests, with <repo root>/eth2.0-spec-tests/ being a git-ignored folder. It's easy to manage the output repo, re-build the tests in there.
    • After rebuilding, it's just a matter of git add . && git commit -S -m "release foo bar" && git push. You activate Git LFS once, and it just works.
  • I'll create an issue to track how this goes, and we can introduce CI based generator triggering later, when we learn how to avoid the pitfalls of maintaining a large-size dynamicly generated repository through user-triggered CI...

What we need after this:

  • Create a eth2.0-spec-tests repository, deprecating the old eth2.0-tests. We start fresh, with YAML files included using LFS, and a good amount of generators. The old eth2.0-tests is deprecated. The naming choice for eth2.0-spec-tests comes from the nature of these tests: they all test spec-behavior. Later on, we may have a eth2.0-network-tests` or others.
    • git lfs install, git lfs track "*.yaml", and commit .gitattributes
    • Add a README.md, LICENSE
  • Update references to old tests, expand testing :)

Copy link
Contributor

@djrtwo djrtwo left a comment

Choose a reason for hiding this comment

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

Looks good. I suppose we'll add notes on how to release generators to readme pending creation of the other repo

@djrtwo djrtwo merged commit b51c788 into dev Apr 22, 2019
@djrtwo djrtwo deleted the generators-workflow branch April 22, 2019 17:08
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.

None yet

2 participants