Provide helper script for template changes and broadcasting #88

sven1103 · 2018-08-08T09:53:41Z

This script should be executed every time a new release is created for nf-core tools. It will traverse all pipelines and make changes according to the cookiecutter template and commit them into a branch called TEMPLATE on each repository.

In addition, a new pull request will be created for every pipeline, which will introduce the changes in the template to the pipeline source code.

This script should be executed, once a new release is done for nf-core tools. It will traverse all pipelines and make changes according to the cookiecutter template and commit them into each pipelines branch 'TEMPLATE'.

sven1103 · 2018-08-08T09:54:22Z

addresses #74

codecov-io · 2018-08-08T09:57:25Z

Codecov Report

Merging #88 into dev will increase coverage by 0.06%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##              dev      #88      +/-   ##
==========================================
+ Coverage   91.47%   91.53%   +0.06%     
==========================================
  Files           7        7              
  Lines         704      709       +5     
==========================================
+ Hits          644      649       +5     
  Misses         60       60

Impacted Files	Coverage Δ
nf_core/lint.py	`96.85% <0%> (+0.05%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1fa5ec8...1d75794. Read the comment docs.

alneberg · 2018-08-08T10:05:02Z

bin/broadcast_prs.py

+        # Get context from pipeline and load it into a dictionary
+        # context = load_context(pipeline)
+        print(pipeline['name']) # Just for testing, can be safely deleted
+        ut.UpdateTemplate(pipeline['name'], context)


Is "ut" defined somewhere?

I think it is a typo. Maybe @sven1103 meant it to be ut = (not ut.) 🤔

copy waste error, had separated test scripts before :D Well spotted!

ewels · 2018-08-08T12:11:44Z

Required config variables for running the cookiecutter template:

manifest.description
manifest.homePage
- To get the pipeline name. Should be in the format 'https://github.com/nf-core/[name]' (this is linted, so tests fail if not)
manifest.nextflowVersion
- New variable that we're just introducing. Will be a string like >=0.31.0 so needs cleaning
- If not present, can use params.nf_required_version which is the older version of the same thing

ewels · 2018-08-08T12:23:48Z

Also, the nf_core.utils.fetch_wf_config function will parse a pipeline config for you, so that's how to fetch the above variables easily.

ewels

Really nice work! A few minor comments and suggestions added here, plus recommendations for updates with the new nf-core create subcommand.

ewels · 2018-08-08T14:56:14Z

bin/broadcast_prs.py

+    def _clone_repo(self):
+        """Clone the repo and switch to the configured branch.
+        """
+        subprocess.run(["git", "clone", self.repo_url, "-b", self.branch, self.pipeline])


This could be nicer with GitPython. Also perhaps working in a temporary directory instead of the cwd? eg:

self.tmpdir = tempfile.mkdtemp() self.repo = git.Repo.clone_from(self.repo_url, self.tmpdir) self.repo.git.checkout(self.branch)

oh yes, looks much cleaner. And it stays in the same thread :D

ewels · 2018-08-08T15:00:40Z

bin/broadcast_prs.py

+        """Apply the changes of the cookiecutter template
+        to the pipelines template branch.
+        """
+        cookiecutter(NF_CORE_TEMPLATE,


Instead of using cookiecutter, you can now use nf-core create:

from nf_core.create import run_cookiecutter run_cookiecutter(self.pipeline_name, self.pipeline_description, self.pipeline_version)

NB: I should probably add an option to specify an output directory. At the moment it just spits it out into the current working directory I think. I guess you'll want to use self.tmpdir.

Also note that currently it creates the files in a subdirectory called self.pipeline_name. I haven't figured an easy way to avoid this, though I guess the create command could also use a temp directory and then copy.

ewels · 2018-08-08T15:05:08Z

bin/broadcast_prs.py

+    def _commit_changes(self):
+        """Commits the changes of the new template to the current branch.
+        """
+        subprocess.run(["git", "add", "-A", "."], cwd=self.pipeline)


self.repo.git.add(A=True) self.repo.index.commit("Update nf-core template")

Could also be nice to reference os.environ('TRAVIS_TAG') in the commit message.

yup, agree, this is a nice idea!

ewels · 2018-08-08T15:05:45Z

bin/broadcast_prs.py

+    Returns: An instance of class requests.Response
+    """
+    content = {}
+    content['title'] = "Important pipeline nf-core update!"


As above, could be nice to reference the release tag with os.environ('TRAVIS_TAG')

ewels · 2018-08-08T15:06:42Z

bin/broadcast_prs.py

+    """
+    content = {}
+    content['title'] = "Important pipeline nf-core update!"
+    content['body'] = "Some important changes have been made in the nf-core pipelines templates.\n" +


We could perhaps link to 'https://github.com/nf-core/tools/releases/tag/{}'.format(os.environ('TRAVIS_TAG')) here too, to easily show the changelog...

ewels · 2018-08-08T15:07:07Z

bin/broadcast_prs.py

+    "Please make sure to merge this in ASAP and make a new minor release of your pipeline."
+    content['head'] = "{}:{}".format(pipeline, template)
+    content['base'] = master
+    return requests.post(url=GITHUB_PR_URL_TEMPL.format(pipeline=pipeline),


This is ridiculously simple and awesome 😎

ewels · 2018-08-08T15:07:16Z

bin/broadcast_prs.py

+
+    Returns: A context dictionary
+    """
+    pass


ewels · 2018-08-08T15:12:05Z

bin/broadcast_prs.py

+    pass
+
+def main():
+    res = requests.get(NF_CORE_PIPELINE_INFO)


Maybe do a check for TRAVIS_TAG environment variable here first to make sure that we're running on Travis?

yup, planned this too :D

alneberg · 2018-08-09T13:41:14Z

I think we need to add the creation of the 'TEMPLATE' branch to the nf-core create command.
This branch also has to be pushed to github, which we could either do within the command or just recommend the user to do.
Currently we only ensure that the first commit is a template-vanilla-only commit.
What do you say?

sven1103 · 2018-08-09T14:04:57Z

@alneberg you are right. But I guess that would be a new issue, as this PR only addresses the distribution, right?

alneberg · 2018-08-10T09:10:24Z

@sven1103 Yes, I totally agree: #125.

Furthermore, can you switch the branch that the PR is issued against from master to dev so that the diff is much smaller and easier to review? Thanks!

alneberg · 2018-08-10T09:38:44Z

I did the switch now. So don't worry about it.

alneberg

Looks really good!

My only concerns are regarding better error catching, especially for pipelines where everything is not yet properly set up.

alneberg · 2018-08-10T09:22:54Z

bin/broadcast_prs.py

+        """
+        self.repo = git.Repo.clone_from(self.repo_url, self.tmpdir)
+        config = utils.fetch_wf_config(wf_path=self.tmpdir)
+        self.repo.git.checkout("origin/{branch}".format(branch=self.branch),


I guess this will fail when pipelines doesn't have a TEMPLATE branch. Might be useful to fail gracefully in that case, since it will be very common at least in the beginning?

I think failing here is totally ok, as the error will be listed in the Travis log anyway with stacktrace. So in my opinion no need to try catch here.

Yes sure, but it would also cause the script to crash on the first pipeline without a TEMPLATE branch and no pull requests at all would be created. But that is perhaps what you intended?

I mean, i can catch the errors in a list, and print them afterwards? So we see all failing pipelines at once?

i think this indeed would make sense here

Sorry, I didn't understand if you changed your mind or not. From my understanding the script will be run once in the tools travis and it would be strange if it would fail/crash until all pipelines are set up properly, right?

I think he changed his mind to agree with you 😉

yup, I agree with @alneberg 's suggestion

alneberg · 2018-08-10T09:40:32Z

bin/broadcast_prs.py

+
+
+def create_pullrequest(pipeline, origin="dev", template="TEMPLATE", token="", user="nf-core"):
+    """Create a pull request to a base branch (default: dev),


I like that it defaults to dev, but I think it is still a lot of pipelines that do not use a dev branch yet.

same as above. Let it fail. The dev branch will be a requirement we need to document :)

ewels · 2018-08-14T08:39:21Z

bin/syncutils/template.py

+        """Apply the changes of the cookiecutter template
+        to the pipelines template branch.
+        """
+        cookiecutter(template_url,


Any reason to do this directly using cookiecutter? It may be safer / more futureproof to use the nf-core.create.run_cookiecutter function instead, in case that is updated in the future... Especially if we add an output directory argument to that feature (which we should - I'll do that now).

yeah, but how can I access this function from within my script? Traversing through the repo just to access the code is quite ugly

I mean, I can extend the system path so python will find the nf-core module, but it is not very consice

ewels

Looking nice! Still some stuff you can simplify if you're using the nf_core command to generate the template.

ewels · 2018-08-14T11:36:42Z

bin/sync

+# The GitHub base url or the nf-core project
+GH_BASE_URL = "https://{token}@github.com/nf-core/{pipeline}"
+# The current cookiecutter template url for nf-core pipelines
+NF_CORE_TEMPLATE = os.path.join(


You don't need to define this any more if you're using the function from the nf_core package.

ewels · 2018-08-14T11:36:58Z

bin/syncutils/template.py

+from cookiecutter.main import cookiecutter
+
+# Enable access to the nf_core package
+rootPath = os.path.abspath("../..")


I don't really understand why you need to do this?

ewels · 2018-08-14T11:38:02Z

bin/syncutils/template.py

+            except:
+                os.remove(os.path.join(target_dir, f))
+        # Move the new template content into the template branch
+        template_path = os.path.join(self.templatedir, self.pipeline)


You don't need to do any of this now that I added the new outdir option for the nf_core function.

Provide helper script for template changes

e531afd

This script should be executed, once a new release is done for nf-core tools. It will traverse all pipelines and make changes according to the cookiecutter template and commit them into each pipelines branch 'TEMPLATE'.

sven1103 added the WIP Work in progress label Aug 8, 2018

Perform some small refactoring

a993d85

alneberg reviewed Aug 8, 2018

View reviewed changes

ewels reviewed Aug 8, 2018

View reviewed changes

sven1103 added 8 commits August 9, 2018 13:59

Refactor a lot of stuff

d495cc0

Merge remote-tracking branch 'upstream/dev'

e9c265e

Merge remote-tracking branch 'upstream/dev'

32a256d

Overwrite template content from new version

575fa2d

Assert for travis tag and add comments

d55023d

Provide Travis Tag in PR

ac8690f

Fix broken markdown syntax

38c7e41

Use bot token for authenticated services

ff8e8a2

Tidy up

a141b3d

alneberg mentioned this pull request Aug 10, 2018

TEMPLATE branch should be set up by the create command #125

Closed

alneberg changed the base branch from master to dev August 10, 2018 09:38

alneberg reviewed Aug 10, 2018

View reviewed changes

ewels added the automation label Aug 10, 2018

sven1103 self-assigned this Aug 13, 2018

sven1103 added 2 commits August 13, 2018 18:35

Refactor sync tools

6baab6b

Introduce better expection handling

da27216

ewels reviewed Aug 14, 2018

View reviewed changes

Use cookiecutter function from nf_core

1d75794

ewels reviewed Aug 14, 2018

View reviewed changes

sven1103 merged commit 1d75794 into nf-core:dev Aug 14, 2018

ewels mentioned this pull request Aug 14, 2018

Refactor sync #137

Merged



		def create_pullrequest(pipeline, origin="dev", template="TEMPLATE", token="", user="nf-core"):
		"""Create a pull request to a base branch (default: dev),

Provide helper script for template changes and broadcasting #88

Provide helper script for template changes and broadcasting #88

Conversation

sven1103 commented Aug 8, 2018 • edited by ewels Loading

sven1103 commented Aug 8, 2018

codecov-io commented Aug 8, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ewels commented Aug 8, 2018

ewels commented Aug 8, 2018

ewels left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alneberg commented Aug 9, 2018

sven1103 commented Aug 9, 2018

alneberg commented Aug 10, 2018

alneberg commented Aug 10, 2018 • edited Loading

alneberg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sven1103 Aug 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sven1103 Aug 14, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ewels left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sven1103 commented Aug 8, 2018 •

edited by ewels

Loading

codecov-io commented Aug 8, 2018 •

edited

Loading

alneberg commented Aug 10, 2018 •

edited

Loading

sven1103 Aug 10, 2018 •

edited

Loading

sven1103 Aug 14, 2018 •

edited

Loading