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

Collection issue: object-oriented task design #18

Open
charlesreid1 opened this issue Sep 11, 2018 · 3 comments
Open

Collection issue: object-oriented task design #18

charlesreid1 opened this issue Sep 11, 2018 · 3 comments

Comments

@charlesreid1
Copy link
Contributor

charlesreid1 commented Sep 11, 2018

We've now reached a point where Uncle Archie hooks are getting a little too complicated.

Solution? Switch to object-oriented programming to simplify our tasks, make things reusable, and cut out all the copypasta that's accumulating.

Ideas:

  • base class: test harness, constructor, setup, teardown, process payload
  • tests will almost always run with a python binary, so for example we can set the python binary, and use it as needed without having to worry about the path being set correctly
  • we need to run some tests in a virtual environment, so have a test harness that sets up a virtual environment and set the python binary correctly

Common functionality:

  • creating a temporary directory
  • checking if pull request, checking if whitelisted repo, etc
  • getting github api instance
  • all of the git cloning and things... (how do we abstract this all away better?)
  • snakemake build
  • updating the commit status
  • assembling a link to the output log
  • assembling a link to the output docs (mkdocs)
  • assembling a link to the snakemake log
  • assembling a report using jinja templates

(Now that we're stepping back and looking at this whole rube goldberg, it's clear it desperately needs to be abstracted.)

Other ideas:

  • could this help with calling the hook functions? factory method?
@charlesreid1
Copy link
Contributor Author

Related:

@charlesreid1
Copy link
Contributor Author

charlesreid1 commented Sep 11, 2018

base classes

  • base class: UncleArchieTask

    • constructor should gather all the params
    • run_cmd should pass command, short description, and cwd
    • process_payload should be implemented as a virtual method
  • base class: GithubTask

    • store API key
    • initialize API instance
    • get repo name (short and full)
    • get PR head commit
    • get PR number
    • check if is PR
    • check if PR being opened
    • check if PR being synced
    • check if PR being merged
  • base class: PythonTask

    • set up virtual environment
    • tear down virtual environment
    • constructor set up
    • destructor tear down

@charlesreid1
Copy link
Contributor Author

charlesreid1 commented Sep 11, 2018

  • base class: PR test

    • process_payload is actually defined, has structure (virtual method, does not need to be defined by derived classes that just want to define test() and have git stuff taken care of)
    • checks (is this a PR, is it being opened/synced, is it against a whitelisted/not-blacklisted base branch)
    • ASSUMPTION FOR NOW: ignore PRs with base branch of gh-pages or heroku-pages (need to go back and fix this to implement a blacklisted branches list)
    • git clone is defined/structured (extract git clone url from payload, clone it to temp working dir)
    • git checkout head commit of PR is defined/structured (extract git sha from payload, from git clone dir)
    • test() function has a defined structure: run test(), return boolean; use boolean to run test_success() or test_fail() (these are virtual methods)
  • derived class: (specific PR test)

    • test() is where the test happens - which is usually just a run command snakemake build
    • test_success() and test_fail() are actually the most complicated - these will write the reports using jinja templates, assemble a final URL, move files to their final htdocs location, mark the commit as pass/fail
  • base class: PR submodule update (generally useful)

    • think through what parameters need to be different here... we specify whitelisted repos, but this particular class and all its children need the user to specify a parent repo at constructor time.
    • has similar structure to the PR run test base class above, but different content
    • process_payload structure is defined, has structure (virtual method, does not need to be defined by derived classes that just want to define test() and have git stuff taken care of)
    • checks (is this a PR merge commit, is it in the whitelisted repos against correct base repo)
    • git clone the parent repo
    • [ ]
  • derived class: PR integration test (generally useful)

    • [ ]
  • derived class: PR build test (generally useful)

    • have set up methods like before
    • define a build() method that runs the build commands
    • parent class defines process_payload() to process the payload if it is a PR
    • parent class checks out head commit
    • parent class calls build() method (virtual method)
    • child classes just just have to just override the build() method (everything else taken care of)
  • derived class: PR mkdocs build test (specific)

    • define build() method to run the mkdocs build test

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

No branches or pull requests

1 participant