[PATCH 1/2] Update the URL to be based upon a /user/copr/<action> structure
by Pierre-Yves Chibon
This is a rather large change which might have consequences in several places. From my
tests it works and hopefully I did not miss to many places.
---
Finally I prefer send the patch up for review.
I managed to get the frontend running pretty quickly, that was very nice ! :)
However, I did not manage to get the tests working easily (I know I could have
dig into the config files to find there the path are hardcoded and adjust them,
but I thought that not being able to run the tests while developing was more a
bug and thus my hack around was not the correct way to approach this).
Thus, I tested my changes against the UI (thus a rather limited test) but I did
not run the unit-tests.
By the way, do you know we have a jenkins instance running in the
infrastructure, would you be interested in hooking up copr's git in it?
The second patch contains changes I made to the unit-tests to adjust the URLs
but as I said, I did not run the tests and therefore there might be tests that
are now failing.
If you are fine with these changes, I will submit patches for the API and adjust
the CLI.
Looking forward for your feedbacks,
Cheers,
Pierre
.../coprs/templates/coprs/_coprs_forms.html | 10 ++++-
coprs_frontend/coprs/templates/coprs/add.html | 2 +-
.../coprs/templates/coprs/detail/overview.html | 7 +++-
coprs_frontend/coprs/templates/coprs/show.html | 6 ++-
.../coprs/views/backend_ns/backend_general.py | 4 ++
.../coprs/views/coprs_ns/coprs_builds.py | 16 +++++---
.../coprs/views/coprs_ns/coprs_general.py | 48 ++++++++++++++--------
coprs_frontend/coprs/views/misc.py | 2 +
8 files changed, 65 insertions(+), 30 deletions(-)
diff --git a/coprs_frontend/coprs/templates/coprs/_coprs_forms.html b/coprs_frontend/coprs/templates/coprs/_coprs_forms.html
index dd13b40..a43d307 100644
--- a/coprs_frontend/coprs/templates/coprs/_coprs_forms.html
+++ b/coprs_frontend/coprs/templates/coprs/_coprs_forms.html
@@ -1,8 +1,14 @@
{% from "_helpers.html" import render_field %}
-{% macro copr_form(form, view, copr = None) %}
+{% macro copr_form(form, view, copr = None, username = None) %}
{# if using for updating, we need to pass name to url_for, but otherwise we need to pass nothing #}
- <form action="{% if copr %}{{ url_for(view, username = copr.owner.name, coprname = copr.name) }}{% else %}{{ url_for(view) }}{% endif %}" method=post class=add-entry>
+ <form action="{% if copr %}
+ {{ url_for(view, username = copr.owner.name, coprname = copr.name) }}
+ {% elif username %}
+ {{ url_for(view, username = username) }}
+ {% else %}
+ {{ url_for(view) }}
+ {% endif %}" method=post class=add-entry>
<dl>
{{ form.csrf_token }}
{{ render_field(form.id, hidden = True) }}
diff --git a/coprs_frontend/coprs/templates/coprs/add.html b/coprs_frontend/coprs/templates/coprs/add.html
index d8a351b..9b97814 100644
--- a/coprs_frontend/coprs/templates/coprs/add.html
+++ b/coprs_frontend/coprs/templates/coprs/add.html
@@ -6,6 +6,6 @@
{% block body %}
<h1>Add a new Copr</h1>
- {{ copr_form(form, view = 'coprs_ns.copr_new') }}
+ {{ copr_form(form, view = 'coprs_ns.copr_new', username=g.user.name) }}
{% endblock %}
diff --git a/coprs_frontend/coprs/templates/coprs/detail/overview.html b/coprs_frontend/coprs/templates/coprs/detail/overview.html
index d924378..fd09f71 100644
--- a/coprs_frontend/coprs/templates/coprs/detail/overview.html
+++ b/coprs_frontend/coprs/templates/coprs/detail/overview.html
@@ -11,7 +11,10 @@
<div class="shift-right">{{ copr.instructions_or_not_filled}}</div>
<h2>Yum repo</h2>
<div class="shift-right">
- <a href="{{ url_for('coprs_ns.generate_repo_file', reponame=copr.owner.name+'-'+copr.name) }}">
+ <a href="{{ url_for(
+ 'coprs_ns.generate_repo_file',
+ username=copr.owner.name,
+ coprname=copr.name) }}">
{{ copr.owner.name }}-{{ copr.name }}.repo</a></div>
<h2>Active Releases</h2>
<table class="releases">
@@ -51,7 +54,7 @@
{% for repo in copr.repos_list %}
<li><a href="{{ repo }}">{{ repo }}</a></li>
{% endfor %}
- </ul>
+ </ul>
{% endif %}
<hr>
diff --git a/coprs_frontend/coprs/templates/coprs/show.html b/coprs_frontend/coprs/templates/coprs/show.html
index 3df2771..e766e48 100644
--- a/coprs_frontend/coprs/templates/coprs/show.html
+++ b/coprs_frontend/coprs/templates/coprs/show.html
@@ -15,7 +15,11 @@
{% endif %}
<div class="coprs-list-{% if g.user %}thin{% else %}thick{% endif %}">
{% if g.user %}
- <div class="add-copr">+ <a href={{ url_for('coprs_ns.copr_add') }}>add a new Copr</a></div>
+ <div class="add-copr">+
+ <a href={{ url_for('coprs_ns.copr_add', username=g.user.name) }}>
+ add a new Copr
+ </a>
+ </div>
{% endif %}
{% if fulltext %}
<div class="search-results">Displaying results for search "{{ fulltext }}"</div>
diff --git a/coprs_frontend/coprs/views/backend_ns/backend_general.py b/coprs_frontend/coprs/views/backend_ns/backend_general.py
index dfdc97d..7d46e2e 100644
--- a/coprs_frontend/coprs/views/backend_ns/backend_general.py
+++ b/coprs_frontend/coprs/views/backend_ns/backend_general.py
@@ -9,6 +9,7 @@ from coprs.logic import builds_logic
from coprs.views import misc
from coprs.views.backend_ns import backend_ns
+
@backend_ns.route('/waiting_builds/')
def waiting_builds():
query = builds_logic.BuildsLogic.get_waiting_builds(None)
@@ -19,6 +20,7 @@ def waiting_builds():
'__included_ids__': False},
'__included_ids__': False}) for build in builds]})
+
@backend_ns.route('/update_builds/', methods = ['POST', 'PUT'])
@misc.backend_authenticated
def update_builds():
@@ -42,12 +44,14 @@ def update_builds():
return flask.jsonify({'updated_builds_ids': list(existing.keys()), 'non_existing_builds_ids': non_existing_ids})
+
@backend_ns.route('/waiting_actions/')
def waiting_actions():
actions = models.Action.query.filter(models.Action.result==helpers.BackendResultEnum('waiting')).all()
return flask.jsonify({'actions': [action.to_dict(options={'__columns_except__': ['result', 'message', 'ended_on']}) for action in actions]})
+
# TODO: this is very similar to update_builds, we should pull out the common functionality into a single function
@backend_ns.route('/update_actions/', methods=['POST', 'PUT'])
@misc.backend_authenticated
diff --git a/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py b/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py
index a6871cd..c8cd0d7 100644
--- a/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py
+++ b/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py
@@ -15,8 +15,9 @@ from coprs.views.misc import login_required, page_not_found
from coprs.views.coprs_ns import coprs_ns
from coprs.views.coprs_ns import coprs_general
-(a)coprs_ns.route('/detail/<username>/<coprname>/builds/', defaults={'page': 1})
-(a)coprs_ns.route('/detail/<username>/<coprname>/builds/<int:page>/')
+
+(a)coprs_ns.route('/<username>/<coprname>/builds/', defaults={'page': 1})
+(a)coprs_ns.route('/<username>/<coprname>/builds/<int:page>/')
def copr_builds(username, coprname, page=1):
copr = coprs_logic.CoprsLogic.get(flask.g.user, username, coprname).first()
@@ -29,7 +30,7 @@ def copr_builds(username, coprname, page=1):
return flask.render_template('coprs/detail/builds.html', copr=copr, builds=paginator.sliced_query, paginator=paginator)
-(a)coprs_ns.route('/detail/<username>/<coprname>/add_build/')
+(a)coprs_ns.route('/<username>/<coprname>/add_build/')
@login_required
def copr_add_build(username, coprname, form=None):
copr = coprs_logic.CoprsLogic.get(flask.g.user, username, coprname).first()
@@ -42,7 +43,8 @@ def copr_add_build(username, coprname, form=None):
return flask.render_template('coprs/detail/add_build.html', copr=copr, form=form)
-(a)coprs_ns.route('/detail/<username>/<coprname>/new_build/', methods = ["POST"])
+
+(a)coprs_ns.route('/<username>/<coprname>/new_build/', methods = ["POST"])
@login_required
def copr_new_build(username, coprname):
form = forms.BuildForm()
@@ -69,7 +71,8 @@ def copr_new_build(username, coprname):
else:
return copr_add_build(username=username, coprname=coprname, form=form)
-(a)coprs_ns.route('/detail/<username>/<coprname>/cancel_build/<int:build_id>/', methods = ['POST'])
+
+(a)coprs_ns.route('/<username>/<coprname>/cancel_build/<int:build_id>/', methods = ['POST'])
@login_required
def copr_cancel_build(username, coprname, build_id):
# only the user who ran the build can cancel it
@@ -86,7 +89,8 @@ def copr_cancel_build(username, coprname, build_id):
return flask.redirect(flask.url_for('coprs_ns.copr_builds', username = username, coprname = coprname))
-(a)coprs_ns.route('/detail/<username>/<coprname>/repeat_build/<int:build_id>/', methods = ['POST'])
+
+(a)coprs_ns.route('/<username>/<coprname>/repeat_build/<int:build_id>/', methods = ['POST'])
@login_required
def copr_repeat_build(username, coprname, build_id):
build = builds_logic.BuildsLogic.get(flask.g.user, build_id).first()
diff --git a/coprs_frontend/coprs/views/coprs_ns/coprs_general.py b/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
index 34f62a1..143ee6c 100644
--- a/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
+++ b/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
@@ -16,6 +16,7 @@ from coprs.views.coprs_ns import coprs_ns
from coprs.logic import builds_logic
from coprs.logic import coprs_logic
+
@coprs_ns.route('/', defaults = {'page': 1})
@coprs_ns.route('/<int:page>/')
def coprs_show(page=1):
@@ -26,8 +27,8 @@ def coprs_show(page=1):
return flask.render_template('coprs/show.html', coprs=coprs, paginator=paginator)
-(a)coprs_ns.route('/owned/<username>/', defaults = {'page': 1})
-(a)coprs_ns.route('/owned/<username>/<int:page>/')
+(a)coprs_ns.route('/<username>/', defaults = {'page': 1})
+(a)coprs_ns.route('/<username>/<int:page>/')
def coprs_by_owner(username=None, page=1):
query = coprs_logic.CoprsLogic.get_multiple(flask.g.user,
user_relation='owned',
@@ -39,8 +40,8 @@ def coprs_by_owner(username=None, page=1):
return flask.render_template('coprs/show.html', coprs=coprs, paginator=paginator)
-(a)coprs_ns.route('/allowed/<username>/', defaults = {'page': 1})
-(a)coprs_ns.route('/allowed/<username>/<int:page>/')
+(a)coprs_ns.route('/<username>/allowed/', defaults = {'page': 1})
+(a)coprs_ns.route('/<username>/allowed/<int:page>/')
def coprs_by_allowed(username=None, page=1):
query = coprs_logic.CoprsLogic.get_multiple(flask.g.user,
user_relation='allowed',
@@ -51,6 +52,7 @@ def coprs_by_allowed(username=None, page=1):
coprs = paginator.sliced_query
return flask.render_template('coprs/show.html', coprs=coprs, paginator=paginator)
+
@coprs_ns.route('/fulltext/', defaults = {'page': 1})
@coprs_ns.route('/fulltext/<int:page>/')
def coprs_fulltext_search(page=1):
@@ -64,17 +66,18 @@ def coprs_fulltext_search(page=1):
paginator=paginator,
fulltext=fulltext)
-(a)coprs_ns.route('/add/')
+
+(a)coprs_ns.route('/<username>/add/')
@login_required
-def copr_add():
+def copr_add(username):
form = forms.CoprFormFactory.create_form_cls()()
return flask.render_template('coprs/add.html', form = form)
-(a)coprs_ns.route('/new/', methods=['POST'])
+(a)coprs_ns.route('/<username>/new/', methods=['POST'])
@login_required
-def copr_new():
+def copr_new(username):
""" Receive information from the user on how to create its new copr
and create it accordingly.
"""
@@ -101,7 +104,8 @@ def copr_new():
else:
return flask.render_template('coprs/add.html', form = form)
-(a)coprs_ns.route('/detail/<username>/<coprname>/')
+
+(a)coprs_ns.route('/<username>/<coprname>/')
def copr_detail(username, coprname):
query = coprs_logic.CoprsLogic.get(flask.g.user, username, coprname, with_mock_chroots=True)
form = forms.CoprLegalFlagForm()
@@ -114,7 +118,8 @@ def copr_detail(username, coprname):
copr=copr,
form=form)
-(a)coprs_ns.route('/detail/<username>/<coprname>/permissions/')
+
+(a)coprs_ns.route('/<username>/<coprname>/permissions/')
def copr_permissions(username, coprname):
query = coprs_logic.CoprsLogic.get(flask.g.user, username, coprname)
copr = query.first()
@@ -145,7 +150,8 @@ def copr_permissions(username, coprname):
permissions = permissions,
current_user_permissions = user_perm)
-(a)coprs_ns.route('/detail/<username>/<coprname>/edit/')
+
+(a)coprs_ns.route('/<username>/<coprname>/edit/')
@login_required
def copr_edit(username, coprname, form=None):
query = coprs_logic.CoprsLogic.get(flask.g.user, username, coprname)
@@ -161,7 +167,7 @@ def copr_edit(username, coprname, form=None):
form=form)
-(a)coprs_ns.route('/detail/<username>/<coprname>/update/', methods = ['POST'])
+(a)coprs_ns.route('/<username>/<coprname>/update/', methods = ['POST'])
@login_required
def copr_update(username, coprname):
form = forms.CoprFormFactory.create_form_cls()()
@@ -189,7 +195,7 @@ def copr_update(username, coprname):
return copr_edit(username, coprname, form)
-(a)coprs_ns.route('/detail/<username>/<coprname>/permissions_applier_change/', methods = ['POST'])
+(a)coprs_ns.route('/<username>/<coprname>/permissions_applier_change/', methods = ['POST'])
@login_required
def copr_permissions_applier_change(username, coprname):
copr = coprs_logic.CoprsLogic.get(flask.g.user, username, coprname).first()
@@ -210,7 +216,8 @@ def copr_permissions_applier_change(username, coprname):
return flask.redirect(flask.url_for('coprs_ns.copr_detail', username = copr.owner.name, coprname = copr.name))
-(a)coprs_ns.route('/detail/<username>/<coprname>/update_permissions/', methods = ['POST'])
+
+(a)coprs_ns.route('/<username>/<coprname>/update_permissions/', methods = ['POST'])
@login_required
def copr_update_permissions(username, coprname):
query = coprs_logic.CoprsLogic.get(flask.g.user, username, coprname)
@@ -238,7 +245,8 @@ def copr_update_permissions(username, coprname):
return flask.redirect(flask.url_for('coprs_ns.copr_detail', username = copr.owner.name, coprname = copr.name))
-(a)coprs_ns.route('/detail/<username>/<coprname>/delete/', methods=['GET', 'POST'])
+
+(a)coprs_ns.route('/<username>/<coprname>/delete/', methods=['GET', 'POST'])
@login_required
def copr_delete(username, coprname):
form = forms.CoprDeleteForm()
@@ -258,7 +266,8 @@ def copr_delete(username, coprname):
else:
return flask.render_template('coprs/detail/delete.html', form=form, copr=copr)
-(a)coprs_ns.route('/detail/<username>/<coprname>/legal_flag/', methods=['POST'])
+
+(a)coprs_ns.route('/<username>/<coprname>/legal_flag/', methods=['POST'])
@login_required
def copr_legal_flag(username, coprname):
form = forms.CoprLegalFlagForm()
@@ -273,14 +282,17 @@ def copr_legal_flag(username, coprname):
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))
-(a)coprs_ns.route('/repo/<reponame>.repo')
-def generate_repo_file(reponame):
+
+(a)coprs_ns.route('/<username>/<coprname>/repo/')
+def generate_repo_file(username, coprname):
''' Generate repo file for a given repo name.
Reponame = username-coprname '''
# This solution is used because flask splits off the last part after a
# dash, therefore user-re-po resolves to user-re/po instead of user/re-po
# FAS usernames may not contain dashes, so this construction is safe.
+ reponame = "%s-%s" % (username, coprname)
+
if '-' not in reponame:
return page_not_found('Bad repository name: {0}. Must be username-coprname'.format(reponame))
diff --git a/coprs_frontend/coprs/views/misc.py b/coprs_frontend/coprs/views/misc.py
index ea79640..9863564 100644
--- a/coprs_frontend/coprs/views/misc.py
+++ b/coprs_frontend/coprs/views/misc.py
@@ -79,6 +79,7 @@ def logout():
flask.flash(u'You were signed out')
return flask.redirect(oid.get_next_url())
+
def api_login_required(f):
@functools.wraps(f)
def decorated_function(*args, **kwargs):
@@ -129,6 +130,7 @@ def login_required(role=helpers.RoleEnum('user')):
else:
return view_wrapper
+
# backend authentication
def backend_authenticated(f):
@functools.wraps(f)
--
1.8.3.1
9 years, 8 months
Pruning policy
by Miroslav Suchý
Hi,
right now COPR keeps all builds. This is not sustainable. Especially
right now, when we lack of disk space.
So we need to have some policy for removing old builds.
What we have on input are:
* successful builds
* failed builds
* builds which succeed in one chroot, but fail in another chroot
Variables for equation are:
* date of the build
* is it last build?
* is it last successful build?
Where we will draw the line? I would suggest:
* Keep everything which was built between NOW() and X days ago.
We can tune up X when we will see how fast we consume disk space. In
ideal world I would imagine X as 14 days. But I can imagine that even
one day would be enough.
* If build is older, keep that build only if it is last successful build
in chroot.
* anything else is going to be deleted. Question: should we keep logs?
This policy can however result in having package foo-1.0 in chroot
fedora-19-i386 and package foo-2.0 in chroot fedora-19-x86_64 (because
foo-2.0 would fail in fedora-19-i386).
Is this what we want?
Ideas?
I would just add that once we migrate to koji as backend (~7 months)
this policy may become obsolete as the retention will be handled by koji.
Mirek
9 years, 8 months
API and unit-tests URL adjustments
by Pierre-Yves Chibon
Hi,
So after breaking the URLs of our dear user, I am now breaking the API to make
it consistent with the UI :)
The first patch changes the URL structure in the API, the second adjust the CLI
to these new URLs.
The third patch was something I ran into while looking if there was any tests
for the API. They are unit-tests for the UI which had still the old URL
structure, so I would expect them to be failing for a little while now.
Was this a known issue (or is it a non-issue)?
Thanks,
Pierre
9 years, 8 months
footer and stylesheet
by Frankie
Hi,
Attached is a patch for the footer and its style sheet.
because I am still working on understanding on how to send patches here
that don't do direct sync i decided to put the files in separate
directories.
This will ensure if it does not work it will not break anything else.
Sorry I took so long for this.
Been a hectic week at work.
Thank you very much.
9 years, 8 months
Status
by Miroslav Suchý
I want to share with you how we are doing:
* I just filed RFR
https://fedoraproject.org/wiki/Infrastructure/RFR/Copr
* I become member of fedora infrastructure team and I'm getting familiar with Fedora apps internals
* Copr is packaged.
* I plan to rebase copr-*-dev very soon and if it will succeed, I will proceed with copr-{be,fe}.
I still hope that by the next week we will open for production.
But there are risks, which may delay deployment:
* I still do not have enough disk space
* Copr still did not pass package review in Fedora
* And we have to get green flag from fedora infrastructure
But we in parallel started working on next phase - integrate koji as backend.
It require that we create namespaces in koji, so you can have two NEVRAs concurrently in Koji.
Mike McLean and James Antill should start on that very soon.
--
Miroslav Suchy, RHCE, RHCDS
Red Hat, Software Engineer, #brno, #devexp, #fedora-buildsys
9 years, 8 months
fedmsg publication
by Ralph Bean
I have a patch[1] here to add fedmsg[2] message publication to the
copr backend. It was written back in March of this year, but I just
recently rebased it to apply against the current git HEAD.
It will publish messages on the following topics:
- build.start
- build.end
- chroot.start
- worker.create
If fedmsg is not installed or is misconfigured, it should run fine and
just print messages that it is having trouble sending messages.
In order to get fedmsg to actually work on our systems, I'll need to
do some configuration in ansible/puppet to:
- Add an /etc/fedmsg.d/ config to the copr backend node
- Create and add copr certs for fedmsg.
- Set up our production bus to receive messages from our cloud nodes.
If you'll accept the patch to copr's source, I'd be glad to carry that
out.
Cheers-
-Ralph
[1] - http://threebean.org/patches/copr-backend-first-pass-at-adding-fedmsg-hoo...
[2] - http://fedmsg.com
9 years, 8 months
Upgrade of Copr without data migration
by Miroslav Suchý
Hi,
I plan - sometimes next week, but no hard date set yet - to upgrade
http://copr-fe.cloud.fedoraproject.org/
and backend as well.
*I have no plan to try to preserve data.*
If this cause problems to you, then please raise your voice and I may do something about it.
Mirek
--
Miroslav Suchy, RHCE, RHCDS
Red Hat, Software Engineer, #brno, #devexp, #fedora-buildsys
9 years, 8 months
Meet docstring standards
by Frankie
Hi guys,
I have attached a patch to assist us in meeting coding and
documentations standards.
Kindly review and let me know if we should proceed to do the same for
the other sections.
Kind Regards,
Onuonga Frankie.
9 years, 8 months
Package review for Fedora
by Miroslav Suchý
Hi,
we are still not ready for production, but I done a lot of packaging work
And from packaging POV the package will not change. At least not significantly.
So I submitted Fedora Package Review:
https://bugzilla.redhat.com/show_bug.cgi?id=1008532
Can somebody pick it up please?
--
Miroslav Suchy, RHCE, RHCDS
Red Hat, Software Engineer, #brno, #devexp, #fedora-buildsys
9 years, 8 months
Copr URLs
by Pierre-Yves Chibon
Hi all,
Something, I have been discussing with Tomas last week but not with
Slavek or the list, is the URLs we have in copr.
For example:
front page: /
detail of a copr: /coprs/detail/toshio/packagedb/
copr's permissions: /coprs/detail/toshio/packagedb/permissions/
copr's builds: /coprs/detail/toshio/packagedb/builds/
someone's copr: /coprs/owned/toshio/
I basically would like to change a couple of things. At the moment when
I'm looking at toshio's packagedb repos there is no way to (from the
url) easily go back to the list of toshio's copr.
So I would like to propose:
/ > home
/coprs/ > list all the repos (same as home, atm)
/coprs/<user>/ > list someone's copr
/coprs/<user>/<repo>/ > the detail of this copr
/coprs/<user>/<repo>/{permissions,builds,...}/ > same as now the
permissions, builds or other page for this repo (the repo file for yum
could end up here as /coprs/<user>/<repo>/repo/ )
It's something I have in mind but would like to discuss before doing
(unless someone beats me to it).
So, what do you think?
Pierre
9 years, 8 months
