Skip to content

Add a webservice for updating feedstocks #115

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

Merged
merged 2 commits into from
Oct 2, 2017

Conversation

jakirkham
Copy link
Member

@jakirkham jakirkham commented Oct 2, 2017

Closes #63

Provides an asynchronous webservice, which listens to push events on the feedstocks. When a push event occurs on a feedstock, the webservice updates the commit that the appropriate submodule points to in the feedstocks repo. This way it is a lot easier to handle updates of various feedstocks throughout the organization on demand instead of running a massive batch script over all of the feedstocks (even when most haven't changed).

Note: This is adapted from the batch feedstock update script.

cc @isuruf

@jakirkham jakirkham force-pushed the add_feedstocks_srv branch 2 times, most recently from bca7129 to dcb3c77 Compare October 2, 2017 00:44
gh = github.Github(os.environ['GH_TOKEN'])
org = gh.get_organization(org_name)

repo_gh = org.get_repo(repo_name)
Copy link
Member

Choose a reason for hiding this comment

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

You can merge the 2 above lines to gh.get_repo("{}/{}".format(org_name, repo_name))

org = gh.get_organization(org_name)

repo_gh = org.get_repo(repo_name)
name = repo_name.rstrip("-feedstock")
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't do what you want.

In [20]: r = "symengine-feedstock"

In [21]: r.rstrip("-feedstock")
Out[21]: 'symengin'

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch! Thanks. Have changed it to r[:-len("-feedstock")], which has the right effect for that case.

feedstock_submodule = feedstocks_repo.submodule(name)
except ValueError:
feedstock_submodule = feedstocks_repo.create_submodule(
name=name,
Copy link
Member Author

Choose a reason for hiding this comment

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

In theory, we should be able to use this even when the feedstock already exists. However it appears that when the feedstock already exists, we get an issue about the name not being defined. 😕 Have raised upstream as issue ( gitpython-developers/GitPython#678 ).

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems that one can hack around this issue by doing feedstock_submodule._name = name.

Copy link
Member Author

Choose a reason for hiding this comment

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

Used the workaround sent upstream in PR ( gitpython-developers/GitPython#679 ). We can drop it after the next gitpython release.

Provides an asynchronous webservice, which listens to push events on the
feedstocks. When a push event occurs on a feedstock, the webservice
updates the commit that the appropriate submodule points to in the
feedstocks repo. This way it is a lot easier to handle updates of
various feedstocks throughout the organization on demand instead of
running a massive batch script over all of the feedstocks (even when
most haven't changed).
Uses a workaround already accepted upstream to fix `create_submodule`
when the submodule already exists. In particular naming the submodule
with the user provided name. By being able to use `create_submodule`,
this script runs substantially faster than when calling `submodule`.
Reason being calling `submodule` lists all submodules in the repo and
then selects the one we named. Whereas `create_submodule` simply
provides us the requested submodule. So using this workaround is
necessary for the performance improvement it provides.

ref: gitpython-developers/GitPython#679
@jakirkham
Copy link
Member Author

Please let me know if there is anything else @isuruf.

Copy link
Member

@isuruf isuruf left a comment

Choose a reason for hiding this comment

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

Have you pushed this into a branch to see if it doesn't break other code?

@jakirkham
Copy link
Member Author

Thanks @isuruf. Good point. Have just done so now.

@jakirkham
Copy link
Member Author

Looks like it is passing. 😄

@jakirkham jakirkham merged commit 7d2d589 into conda-forge:master Oct 2, 2017
@jakirkham jakirkham deleted the add_feedstocks_srv branch October 2, 2017 15:59
@jakirkham
Copy link
Member Author

This is good timing to as the feedstock update script has been down for the past couple of days. Likely due to the sheer number of feedstocks it is trying to update.


gh = github.Github(os.environ['GH_TOKEN'])

gh.get_repo("{}/{}".format(org_name, repo_name))
Copy link
Member Author

Choose a reason for hiding this comment

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

Somehow missed storing this anywhere. 😱 Fixing in PR ( #117 ).


# Submit changes
if feedstocks_repo.is_dirty():
feedstocks_repo.index.commit("Updated feedstocks submodules. [ci skip]")
Copy link
Member Author

Choose a reason for hiding this comment

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

Was hoping the GitHub token would be enough to get the author info set correctly, but appears that is not the case. Adding author details in PR ( #118 ).

Copy link
Member

Choose a reason for hiding this comment

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

Would be nice if the commit message had the repo name.

Copy link
Member Author

Choose a reason for hiding this comment

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

Was thinking about that also. Didn't want to deviate from the feedstock update script too much (at least not in behavior) initially. Though once we get rid of that script, seems reasonable to update this to be more flexible.

Copy link
Member Author

Choose a reason for hiding this comment

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

PR ( #134 ) should do the trick.


# Submit changes
if feedstocks_repo.is_dirty():
feedstocks_repo.index.commit("Updated feedstocks submodules. [ci skip]")
Copy link
Member Author

Choose a reason for hiding this comment

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

Need to manually stage changes as well for commit to work. Addressed in PR ( #119 ).

@jakirkham
Copy link
Member Author

jakirkham commented Oct 2, 2017

Took a few follow-up PRs, but it appears it is now working. 🎉

Might be worth adding a command work to update the feedstocks repo if it is out of date. ( #120 )

@jakirkham
Copy link
Member Author

Partially addresses issue ( conda-forge/conda-forge.github.io#214 ).

parser.add_argument('org')
parser.add_argument('repo')
args = parser.parse_args()
update_team(args.org, args.repo)
Copy link
Member Author

Choose a reason for hiding this comment

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

Should have been update_feedstock. Fixing in PR ( #135 ).

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