Hi,
I fixed the SQL query, please comment before I push. I also have a design-related question, if I may.
Currently, the baseurl in the .repo file points to the root of the repo. However, if a build is done on multiple architectures, will that work, or do I need a baseurl pointing to that architecture subdirectory? Based on the answer, I will also implement a link from the copr overview page to the respective .repo files.
Thank you, TR
Patch:
diff --git a/coprs_frontend/coprs/templates/coprs/coprchroot.repo b/coprs_frontend/coprs/templates/coprs/coprchroot.repo new file mode 100644 index 0000000..98d79d3 --- /dev/null +++ b/coprs_frontend/coprs/templates/coprs/coprchroot.repo @@ -0,0 +1,7 @@ +[{{ copr.owner.name }}-{{ copr.name }}] + +name=Copr repo for {{ copr.name }} owned by {{ copr.owner.name }} +description={{ copr.description }} +baseurl={{ url }} +skip_if_unavailable=True + diff --git a/coprs_frontend/coprs/views/coprs_ns/coprs_general.py b/coprs_frontend/coprs/views/coprs_ns/coprs_general.py index 9d5be98..7cc83c5 100644 --- a/coprs_frontend/coprs/views/coprs_ns/coprs_general.py +++ b/coprs_frontend/coprs/views/coprs_ns/coprs_general.py @@ -272,3 +272,23 @@ def copr_legal_flag(username, coprname): db.session.commit() flask.flash('Admin was noticed about your report and will investigate the copr shortly.') return flask.redirect(flask.url_for('coprs_ns.copr_detail', username=username, coprname=coprname)) + +@coprs_ns.route('/detail/<username>/<coprname>/<chrootname>.repo', methods = ['GET']) +def generate_repo_file(username, coprname, chrootname): + ''' Generate repo file for a given copr/mock-chroot ''' + copr = None + try: + # query.one() is used since it fetches all builds, unlike query.first(). + copr = coprs_logic.CoprsLogic.get(flask.g.user, username, coprname, + with_mock_chroots=True, with_builds=True).one() + except sqlalchemy.orm.exc.NoResultFound: + return page_not_found('Copr: {0}/{1} does not exist'.format(username, coprname)) + + urls = [build.results for build in copr.builds if build.results != None] + if not urls: + return page_not_found('Repository not initialized: No builds in {0}/{1} have finished.'.format(username, coprname)) + + response = flask.make_response(flask.render_template('coprs/coprchroot.repo', copr=copr, url=urls[0])) + response.mimetype='text/plain' + return response + diff --git a/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_general.py b/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_general.py index 380d59d..e70458b 100644 --- a/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_general.py +++ b/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_general.py @@ -1,4 +1,5 @@ import flask +import pytest
from flexmock import flexmock
@@ -456,3 +457,30 @@ class TestCoprDelete(CoprsTestCase): assert 'Copr was deleted successfully' not in r.data assert not self.models.Action.query.first() assert self.models.Copr.query.filter(self.models.Copr.id==self.c1.id).first() + +class TestCoprRepoGeneration(CoprsTestCase): + @pytest.fixture + def f_custom_builds(self): + ''' Custom builds are used in order not to break the default ones ''' + self.b5 = self.models.Build(copr=self.c1, user=self.u1, chroots='fedora-18-x86_64', submitted_on=9, ended_on=200, results='foo://bar.baz', status=1) + self.b6 = self.models.Build(copr=self.c1, user=self.u1, chroots='fedora-18-x86_64', submitted_on=11) + self.b7 = self.models.Build(copr=self.c1, user=self.u1, chroots='fedora-18-x86_64', submitted_on=10, ended_on=150, results='foo://bar.baz', status=1) + + self.db.session.add_all([self.b5, self.b6, self.b7]) + + def test_fail_on_no_copr(self): + r = self.tc.get('/coprs/detail/bogus/copr/name.repo') + assert r.status_code == 404 + assert 'bogus/copr does not exist' in r.data + + def test_fail_on_no_finished_builds(self, f_users, f_coprs, f_mock_chroots, f_db): + r = self.tc.get( + '/coprs/detail/{0}/{1}/{2}.repo'.format(self.u1.name, self.c1.name, + self.c1.mock_chroots[0].chroot_name)) + assert r.status_code == 404 + assert 'Repository not initialized' in r.data + + def test_works_on_older_builds(self, f_users, f_coprs, f_mock_chroots, f_custom_builds, f_db): + r = self.tc.get('/coprs/detail/{0}/{1}/{2}.repo'.format(self.u1.name, self.c1.name, self.c1.mock_chroots[0].chroot_name)) + assert r.status_code == 200 + assert 'baseurl=foo://bar.baz' in r.data
Hi, thanks for your effor Tomas, please see few comments below:
----- Original Message -----
Hi,
I fixed the SQL query, please comment before I push. I also have a design-related question, if I may.
Currently, the baseurl in the .repo file points to the root of the repo. However, if a build is done on multiple architectures, will that work, or do I need a baseurl pointing to that architecture subdirectory? Based on the answer, I will also implement a link from the copr overview page to the respective .repo files.
Seth would probably know this best. Seth?
Thank you, TR
<snip>
+@coprs_ns.route('/detail/<username>/<coprname>/<chrootname>.repo', methods = ['GET'])
- methods = ['GET'] is default, you don't need to specify that explicitly - If I get it correctly, the .repo filename is only chroot_name? That would mean something like "fedora-18-x86_64.repo"? If so, this would only allow you to have one copr in your yum.repos.d, right?
<snip>
- urls = [build.results for build in copr.builds if build.results != None]
- I don't really like this. This will take _all_ the builds and traverse and create a list of possibly all of them (since you can assume that almost all builds are complete after the copr has lived for some time). I'd suggest using a generator with the same expression and use it to traverse builds - something along the lines of:
urls = (<the same that you have>) for u in urls: if u: break else: # all traversed, but no finished build found => page_not_found
# if we get here, u is a finished build => render repofile
- if not urls:
return page_not_found('Repository not initialized: No builds in
{0}/{1} have finished.'.format(username, coprname))
- response =
flask.make_response(flask.render_template('coprs/coprchroot.repo', copr=copr, url=urls[0]))
- response.mimetype='text/plain'
- return response
Could you please fix these nits before committing? Thanks a lot, Slavek.
Hi,
On Mon, 27 May 2013 08:07:44 -0400 (EDT) Bohuslav Kabrda bkabrda@redhat.com wrote:
+@coprs_ns.route('/detail/<username>/<coprname>/<chrootname>.repo', methods = ['GET'])
- methods = ['GET'] is default, you don't need to specify that explicitly
- If I get it correctly, the .repo filename is only chroot_name? That would mean something like "fedora-18-x86_64.repo"? If so, this would only allow you to have one copr in your yum.repos.d, right?
Well, the idea was that you probably won't want to have 2 repos of the same name from two different people, but if you do, I can easily change it to point to <username>-<coprname>-<chrootname>.repo or the like. State your preferences, please, because this is merely a matter of specification.
- urls = [build.results for build in copr.builds if build.results != None]
- I don't really like this. This will take _all_ the builds and traverse and create a list of possibly all of them (since you can assume that almost all builds are complete after the copr has lived for some time). I'd suggest using a generator with the same expression and use it to traverse builds - something along the lines of:
urls = (<the same that you have>) for u in urls: if u: break else: # all traversed, but no finished build found => page_not_found
# if we get here, u is a finished build => render repofile
- if not urls:
return page_not_found('Repository not initialized: No builds in
{0}/{1} have finished.'.format(username, coprname))
- response =
flask.make_response(flask.render_template('coprs/coprchroot.repo', copr=copr, url=urls[0]))
- response.mimetype='text/plain'
- return response
Done.
Could you please fix these nits before committing?
I will delay committing until I get a final word on how the (base)URL is supposed to look like.
Regards, TR
-- Regards, Bohuslav "Slavek" Kabrda. _______________________________________________ copr-devel mailing list copr-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/copr-devel
----- Original Message -----
Hi,
On Mon, 27 May 2013 08:07:44 -0400 (EDT) Bohuslav Kabrda bkabrda@redhat.com wrote:
+@coprs_ns.route('/detail/<username>/<coprname>/<chrootname>.repo', methods = ['GET'])
- methods = ['GET'] is default, you don't need to specify that explicitly
- If I get it correctly, the .repo filename is only chroot_name? That would
mean something like "fedora-18-x86_64.repo"? If so, this would only allow you to have one copr in your yum.repos.d, right?
Well, the idea was that you probably won't want to have 2 repos of the same name from two different people, but if you do, I can easily change it to point to <username>-<coprname>-<chrootname>.repo or the like. State your preferences, please, because this is merely a matter of specification.
The idea is exactly that you can have 2 repos of the same name for different people :)
<snip other comments>
Done.
Could you please fix these nits before committing?
I will delay committing until I get a final word on how the (base)URL is supposed to look like.
Sure, thanks. Slavek.
Regards, TR
copr-devel@lists.fedorahosted.org