[PATCH 1/3] Make the api return 500 status code when something goes wrong.

[Pierre-Yves Chibon]

---
 coprs_frontend/coprs/views/api_ns/api_general.py | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/coprs_frontend/coprs/views/api_ns/api_general.py
b/coprs_frontend/coprs/views/api_ns/api_general.py
index 735da98..7c87388 100644
--- a/coprs_frontend/coprs/views/api_ns/api_general.py
+++ b/coprs_frontend/coprs/views/api_ns/api_general.py
@@ -53,6 +53,7 @@ def api_new_copr():
 
     """
     form = forms.CoprFormFactory.create_form_cls()(csrf_enabled=False)
+    httpcode = 200
     if form.validate_on_submit():
         infos = []
         try:
@@ -79,14 +80,20 @@ def api_new_copr():
             db.session.commit()
         except exceptions.DuplicateException, err:
             output = {'output': 'notok', 'error': err}
+            httpcode = 500
             db.session.rollback()
 
     else:
-        errormsg = "\n".join(form.errors['name'])
+        errormsg = ''
+        if form.errors:
+            errormsg = "\n".join(form.errors['name'])
         errormsg = errormsg.replace('"', "'")
         output = {'output': 'notok', 'error': errormsg}
+        httpcode = 500
 
-    return flask.jsonify(output)
+    jsonout = flask.jsonify(output)
+    jsonout.status_code = httpcode
+    return jsonout
 
 
 @api_ns.route('/owned/')
@@ -101,6 +108,7 @@ def api_coprs_by_owner(username=None):
 
     """
     username = flask.request.args.get('username', None) or username
+    httpcode = 200
     if 'username':
         query = coprs_logic.CoprsLogic.get_multiple(flask.g.user,
             user_relation='owned', username=username)
@@ -113,5 +121,8 @@ def api_coprs_by_owner(username=None):
                                     'instructions': repo.instructions})
     else:
         output = {'output': 'notok', 'error': 'Invalid
request'}
+        httpcode = 500
 
-    return flask.jsonify(output)
+    jsonout = flask.jsonify(output)
+    jsonout.status_code = httpcode
+    return jsonout
-- 
1.8.1

[PATCH2/3]AddtheAPIentrytocreateanewbuildinacoprrepository

[Pierre-Yves Chibon]

---

Although this is looking fine especially since it is so close from the code in
the normal controler, I have not tested it, so I guess it might break, future
patch will come if so :)

 coprs_frontend/coprs/views/api_ns/api_general.py | 35 ++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/coprs_frontend/coprs/views/api_ns/api_general.py
b/coprs_frontend/coprs/views/api_ns/api_general.py
index 7c87388..4f9aae5 100644
--- a/coprs_frontend/coprs/views/api_ns/api_general.py
+++ b/coprs_frontend/coprs/views/api_ns/api_general.py
@@ -126,3 +126,38 @@ def api_coprs_by_owner(username=None):
     jsonout = flask.jsonify(output)
     jsonout.status_code = httpcode
     return jsonout
+
+
+@api_ns.route('/coprs/detail/<username>/<coprname>/new_build/',
+    methods = ["POST"])
+@login_required
+def copr_new_build(username, coprname):
+    form = forms.BuildForm()
+    copr = coprs_logic.CoprsLogic.get(flask.g.user, username, coprname).first()
+    httpcode = 200
+    if not copr:
+        output = {'output': 'notok', 'error':
+            'Copr with name {0} does not exist.'.format(coprname)}
+        httpcode = 500
+
+    else:
+        if form.validate_on_submit() and flask.g.user.can_build_in(copr):
+            # we're checking authorization above for now
+            build = builds_logic.BuildsLogic.add(user=flask.g.user,
+                pkgs=form.pkgs.data.replace('\n', ' '), copr=copr)
+
+            if flask.g.user.proven:
+                build.memory_reqs = form.memory_reqs.data
+                build.timeout = form.timeout.data
+
+            db.session.commit()
+
+            output = {'output': 'ok', 'message':
+                'Build was added to {0}.'.format(coprname)}
+        else:
+            output = {'output': 'notok', 'error': 'Invalid
request'}
+            httpcode = 500
+
+    jsonout = flask.jsonify(output)
+    jsonout.status_code = httpcode
+    return jsonout
-- 
1.8.1

[PATCH2/3]AddtheAPIentrytocreateanewbuildinacoprrepository

[Pierre-Yves Chibon]

On Sat, 2013-02-02 at 17:53 +0100, Pierre-Yves Chibon wrote:
>  ---
>  
>  Although this is looking fine especially since it is so close from the code in
>  the normal controler, I have not tested it, so I guess it might break, future
>  patch will come if so :) 
I'm thinking, let's not integrate this patch yet and merge it only when
I will have tested it.

This was more meant to give an idea on what it will look like.

I'll resend a patch for using the 'Authorization' header so that this
one at least can be applied.

Pierre

[PATCH2/3]AddtheAPIentrytocreateanewbuildinacoprrepository

[Pierre-Yves Chibon]

On Sun, 2013-02-03 at 20:43 +0100, Pierre-Yves Chibon wrote:
>  On Sat, 2013-02-02 at 17:53 +0100, Pierre-Yves Chibon wrote:
>  > ---
>  > 
>  > Although this is looking fine especially since it is so close from the code in
>  > the normal controler, I have not tested it, so I guess it might break, future
>  > patch will come if so :)
>  
>  I'm thinking, let's not integrate this patch yet and merge it only when
>  I will have tested it.
>  
>  This was more meant to give an idea on what it will look like.
>  
>  I'll resend a patch for using the 'Authorization' header so that this
>  one at least can be applied. 
Ok the week-end has been long... I meant this for the commit integrating
the 'build-copr' action in the CLI.

Sorry for the confusion.

Pierre

[PATCH2/3]AddtheAPIentrytocreateanewbuildinacoprrepository

[Bohuslav Kabrda]

----- Original Message -----
>  On Sun, 2013-02-03 at 20:43 +0100, Pierre-Yves Chibon wrote:
>  > On Sat, 2013-02-02 at 17:53 +0100, Pierre-Yves Chibon wrote:
>  > > ---
>  > > 
>  > > Although this is looking fine especially since it is so close
>  > > from the code in
>  > > the normal controler, I have not tested it, so I guess it might
>  > > break, future
>  > > patch will come if so :)
>  > 
>  > I'm thinking, let's not integrate this patch yet and merge it only
>  > when
>  > I will have tested it.
>  > 
>  > This was more meant to give an idea on what it will look like.
>  > 
>  > I'll resend a patch for using the 'Authorization' header so that
>  > this
>  > one at least can be applied.
>  
>  Ok the week-end has been long... I meant this for the commit
>  integrating
>  the 'build-copr' action in the CLI.
>  
>  Sorry for the confusion.
>  
>  Pierre 
Ok, I hope I applied everything that I should have applied. If not, give me a shout :)

-- 
Regards,
Bohuslav "Slavek" Kabrda.

[PATCH2/3]AddtheAPIentrytocreateanewbuildinacoprrepository

[Pierre-Yves Chibon]

On Mon, 2013-02-04 at 06:03 -0500, Bohuslav Kabrda wrote:
>  ----- Original Message -----
>  > On Sun, 2013-02-03 at 20:43 +0100, Pierre-Yves Chibon wrote:
>  > > On Sat, 2013-02-02 at 17:53 +0100, Pierre-Yves Chibon wrote:
>  > > > ---
>  > > > 
>  > > > Although this is looking fine especially since it is so close
>  > > > from the code in
>  > > > the normal controler, I have not tested it, so I guess it might
>  > > > break, future
>  > > > patch will come if so :)
>  > > 
>  > > I'm thinking, let's not integrate this patch yet and merge it only
>  > > when
>  > > I will have tested it.
>  > > 
>  > > This was more meant to give an idea on what it will look like.
>  > > 
>  > > I'll resend a patch for using the 'Authorization' header so that
>  > > this
>  > > one at least can be applied.
>  > 
>  > Ok the week-end has been long... I meant this for the commit
>  > integrating
>  > the 'build-copr' action in the CLI.
>  > 
>  Ok, I hope I applied everything that I should have applied. If not, give me a shout :)

Looks right to me, thanks for pushing all this :)

When I'll work again on the CLI (some evening this week I guess), I'll
need to pull on the server, is this fine for everyone ?


Thanks,
Pierre

[PATCH2/3]AddtheAPIentrytocreateanewbuildinacoprrepository

[Bohuslav Kabrda]

----- Original Message -----
>  On Mon, 2013-02-04 at 06:03 -0500, Bohuslav Kabrda wrote:
>  > ----- Original Message -----
>  > > On Sun, 2013-02-03 at 20:43 +0100, Pierre-Yves Chibon wrote:
>  > > > On Sat, 2013-02-02 at 17:53 +0100, Pierre-Yves Chibon wrote:
>  > > > > ---
>  > > > > 
>  > > > > Although this is looking fine especially since it is so close
>  > > > > from the code in
>  > > > > the normal controler, I have not tested it, so I guess it
>  > > > > might
>  > > > > break, future
>  > > > > patch will come if so :)
>  > > > 
>  > > > I'm thinking, let's not integrate this patch yet and merge it
>  > > > only
>  > > > when
>  > > > I will have tested it.
>  > > > 
>  > > > This was more meant to give an idea on what it will look like.
>  > > > 
>  > > > I'll resend a patch for using the 'Authorization' header so
>  > > > that
>  > > > this
>  > > > one at least can be applied.
>  > > 
>  > > Ok the week-end has been long... I meant this for the commit
>  > > integrating
>  > > the 'build-copr' action in the CLI.
>  > > 
>  > Ok, I hope I applied everything that I should have applied. If not,
>  > give me a shout :)
>  
>  
>  Looks right to me, thanks for pushing all this :)
>  
>  When I'll work again on the CLI (some evening this week I guess),
>  I'll
>  need to pull on the server, is this fine for everyone ?
>  
>  
>  Thanks,
>  Pierre 
I'll be pulling today since I want to finally get the fulltext search working.
It'll require some DB updates etc, so please wait until I pull. I'll let you
know.
Thanks.

-- 
Regards,
Bohuslav "Slavek" Kabrda.

[PATCH2/3]AddtheAPIentrytocreateanewbuildinacoprrepository

[Seth Vidal]

On Mon, 4 Feb 2013, Bohuslav Kabrda wrote:

> >> Ok, I hope I applied everything that I should have applied.
> If not,
> >> give me a shout :)
> >
> >
> > Looks right to me, thanks for pushing all this :)
> >
> > When I'll work again on the CLI (some evening this week I guess),
> > I'll
> > need to pull on the server, is this fine for everyone ?
> >
> >
> > Thanks,
> > Pierre
>
>  I'll be pulling today since I want to finally get the fulltext search working.
> It'll require some DB updates etc, so please wait until I pull. I'll let you
> know.
>  Thanks.
>  
Okay - I'm pretty sure the 500 issue is breaking the backend url for 
getting the builds.

I'm seeing this in the copr backend logs from the job fetcher:


: jobgrab: Error retrieving jobs from http://copr-fe.cloud.fedoraproject.org/backend:
HTTPConnectionPool(host='copr-fe.cloud.fedoraproject.org', port=80): Max 
retries exceeded with url: /backend/waiting_builds/


the url seems to WORK - but I can't tell where the errors are coming from.

-sv

[PATCH2/3]AddtheAPIentrytocreateanewbuildinacoprrepository

[Bohuslav Kabrda]

----- Original Message -----
>  
>  
>  
>  On Mon, 4 Feb 2013, Bohuslav Kabrda wrote:
>  
>  >>> Ok, I hope I applied everything that I should have applied. If
>  >>> not,
>  >>> give me a shout :)
>  >>
>  >>
>  >> Looks right to me, thanks for pushing all this :)
>  >>
>  >> When I'll work again on the CLI (some evening this week I guess),
>  >> I'll
>  >> need to pull on the server, is this fine for everyone ?
>  >>
>  >>
>  >> Thanks,
>  >> Pierre
>  >
>  > I'll be pulling today since I want to finally get the fulltext
>  > search working. It'll require some DB updates etc, so please wait
>  > until I pull. I'll let you know.
>  > Thanks.
>  >
>  
>  Okay - I'm pretty sure the 500 issue is breaking the backend url for
>  getting the builds.
>  
>  I'm seeing this in the copr backend logs from the job fetcher:
>  
>  
>  : jobgrab: Error retrieving jobs from
>  http://copr-fe.cloud.fedoraproject.org/backend:
>  HTTPConnectionPool(host='copr-fe.cloud.fedoraproject.org', port=80):
>  Max
>  retries exceeded with url: /backend/waiting_builds/
>  
>  
>  the url seems to WORK - but I can't tell where the errors are coming
>  from.
>  
>  -sv
>  
>   
Hmm, no clue what that might be caused by. The last 500 on /backend/waiting_builds/ in
apache access log is from [24/Jan/2013:12:40:49 +0000] - this has already been repaired.
When is the last time you got 500?

-- 
Regards,
Bohuslav "Slavek" Kabrda.

[PATCH3/3]Smallpep8cleanupontheAPIcode

[Pierre-Yves Chibon]

---
 coprs_frontend/coprs/views/api_ns/api_general.py | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/coprs_frontend/coprs/views/api_ns/api_general.py
b/coprs_frontend/coprs/views/api_ns/api_general.py
index 4f9aae5..d12ea7e 100644
--- a/coprs_frontend/coprs/views/api_ns/api_general.py
+++ b/coprs_frontend/coprs/views/api_ns/api_general.py
@@ -24,13 +24,14 @@ def api_home():
     return flask.render_template('api.html')
 
 
-(a)api_ns.route('/new/', methods = ["GET", "POST"])
+(a)api_ns.route('/new/', methods=["GET", "POST"])
 @login_required
 def api_new_token():
     """ Method use to generate a new API token for the current user.
     """
     user = flask.g.user
-    user.api_token =
helpers.generate_api_token(flask.current_app.config['API_TOKEN_LENGTH'])
+    user.api_token = helpers.generate_api_token(
+        flask.current_app.config['API_TOKEN_LENGTH'])
     user.api_token_expiration = datetime.date.today() \
         + datetime.timedelta(days=30)
     db.session.add(user)
@@ -64,8 +65,7 @@ def api_new_copr():
                 selected_chroots=form.selected_chroots,
                 description=form.description.data,
                 instructions=form.instructions.data,
-                check_for_duplicates=True,
-                )
+                check_for_duplicates=True)
             infos.append('New copr was successfully created.')
 
             if form.initial_pkgs.data:
@@ -73,10 +73,10 @@ def api_new_copr():
                     pkgs=" ".join(form.initial_pkgs.data.split()),
                     copr=copr,
                     owner=flask.g.user)
-                infos.append('Initial packages were successfully submitted '
-                        'for building.')
+                infos.append('Initial packages were successfully '
+                    'submitted for building.')
 
-            output = {'output': 'ok', 'message' :
'\n'.join(infos)}
+            output = {'output': 'ok', 'message':
'\n'.join(infos)}
             db.session.commit()
         except exceptions.DuplicateException, err:
             output = {'output': 'notok', 'error': err}
@@ -129,11 +129,12 @@ def api_coprs_by_owner(username=None):
 
 
 @api_ns.route('/coprs/detail/<username>/<coprname>/new_build/',
-    methods = ["POST"])
+    methods=["POST"])
 @login_required
 def copr_new_build(username, coprname):
     form = forms.BuildForm()
-    copr = coprs_logic.CoprsLogic.get(flask.g.user, username, coprname).first()
+    copr = coprs_logic.CoprsLogic.get(flask.g.user, username,
+        coprname).first()
     httpcode = 200
     if not copr:
         output = {'output': 'notok', 'error':
-- 
1.8.1

[PATCH1/2]pep8runonthesubcommand.pyfile

[Pierre-Yves Chibon]

---
 copr_cli/subcommands.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/copr_cli/subcommands.py b/copr_cli/subcommands.py
index b91ce0c..ce267a2 100644
--- a/copr_cli/subcommands.py
+++ b/copr_cli/subcommands.py
@@ -19,7 +19,7 @@ def set_user():
     """ Retrieve the user information from the config file.
"""
     config = ConfigParser.ConfigParser()
     config.read(os.path.join(os.path.expanduser('~'), '.config',
-        'copr'))
+                'copr'))
     username = config.get('copr-cli', 'username', None)
     token = config.get('copr-cli', 'token', None)
     return {'username': username, 'token': token}
@@ -53,7 +53,7 @@ class List(cliff.lister.Lister):
             else:
                 columns = ['output']
                 values = ['No copr retrieved for user: "{0}"'.format(
-                            user['username'])]
+                    user['username'])]
                 return (columns, [values])
         else:
             columns = ['output']
@@ -97,7 +97,7 @@ class AddCopr(Command):
                 'initial_pkgs': initial_pkgs,
                 'description': args.description,
                 'instructions': args.instructions
-        }
+                }
         for chroot in args.chroots:
             data[chroot] = 'y'
 
-- 
1.8.1

[PATCH2/2]Implementthebuildactioninthecopr-cli

[Pierre-Yves Chibon]

The build action is used to build certain packages in a given copr.
---

Ok this is a first attempt to the build-copr action in the CLI.

Again, not tested :-)

 copr_cli/subcommands.py | 36 ++++++++++++++++++++++++++++++++++++
 coprcli-setup.py        |  1 +
 2 files changed, 37 insertions(+)

diff --git a/copr_cli/subcommands.py b/copr_cli/subcommands.py
index ce267a2..7264250 100644
--- a/copr_cli/subcommands.py
+++ b/copr_cli/subcommands.py
@@ -107,3 +107,39 @@ class AddCopr(Command):
             print output['message']
         else:
             print 'Something went wrong:\n  {0}'.format(output['error'])
+
+
+class Build(Command):
+    """ Build a new package into a given copr. """
+
+    log = logging.getLogger(__name__)
+
+    def get_parser(self, prog_name):
+        parser = super(type(self), self).get_parser(prog_name)
+        parser.add_argument('copr')
+        parser.add_argument('names', nargs='?',
+            action='append')
+        parser.add_argument('--memory', dest='memory',
+                            help="")
+        parser.add_argument('--timeout', dest='timeout',
+                            help="")
+        return parser
+
+    def take_action(self, args):
+        user = set_user()
+        URL = '{0}/coprs/detail/{1}/{2}/new_build/'.format(
+            copr_api_url,
+            user['username'],
+            args.copr)
+
+        data = {'pkgs': ' '.join(args.names),
+                'memory': args.memory,
+                'timeout': args.timeout
+                }
+
+        req = requests.post(URL, params=user, data=data)
+        output = json.loads(req.text)
+        if req.status_code == 200:
+            print 'Something went wrong:\n  {0}'.format(output['error'])
+        else:
+            print 'Build submitted'
diff --git a/coprcli-setup.py b/coprcli-setup.py
index a7a40d3..0084494 100644
--- a/coprcli-setup.py
+++ b/coprcli-setup.py
@@ -30,6 +30,7 @@ requires = [
 subcommands = [
     'list = copr_cli.subcommands:List',
     'add-copr = copr_cli.subcommands:AddCopr',
+    'build-copr = copr_cli.subcommands:Build',
 ]
 
 __name__ = 'copr-cli'
-- 
1.8.1

[PATCH]Implementthebuildactioninthecopr-cli

[Pierre-Yves Chibon]

The build action is used to build certain packages in a given copr.
---

Ok, this is a better version of the previous patch.
It handles better the multiple packages passed as argument (and we need at least
one package to build).
I also renamed the positional argument 'names' to 'pkgs' to make it more
consistent with what it represents and the API/webapp.

 copr_cli/subcommands.py | 35 +++++++++++++++++++++++++++++++++++
 coprcli-setup.py        |  1 +
 2 files changed, 36 insertions(+)

diff --git a/copr_cli/subcommands.py b/copr_cli/subcommands.py
index ce267a2..cb112f2 100644
--- a/copr_cli/subcommands.py
+++ b/copr_cli/subcommands.py
@@ -107,3 +107,38 @@ class AddCopr(Command):
             print output['message']
         else:
             print 'Something went wrong:\n  {0}'.format(output['error'])
+
+
+class Build(Command):
+    """ Build a new package into a given copr. """
+
+    log = logging.getLogger(__name__)
+
+    def get_parser(self, prog_name):
+        parser = super(type(self), self).get_parser(prog_name)
+        parser.add_argument('copr')
+        parser.add_argument('pkgs', nargs='+', action='append')
+        parser.add_argument('--memory', dest='memory',
+                            help="")
+        parser.add_argument('--timeout', dest='timeout',
+                            help="")
+        return parser
+
+    def take_action(self, args):
+        user = set_user()
+        URL = '{0}/coprs/detail/{1}/{2}/new_build/'.format(
+            copr_api_url,
+            user['username'],
+            args.copr)
+
+        data = {'pkgs': ' '.join(args.pkgs[0]),
+                'memory': args.memory,
+                'timeout': args.timeout
+                }
+
+        req = requests.post(URL, params=user, data=data)
+        output = json.loads(req.text)
+        if req.status_code == 200:
+            print 'Something went wrong:\n  {0}'.format(output['error'])
+        else:
+            print 'Build submitted'
diff --git a/coprcli-setup.py b/coprcli-setup.py
index a7a40d3..0084494 100644
--- a/coprcli-setup.py
+++ b/coprcli-setup.py
@@ -30,6 +30,7 @@ requires = [
 subcommands = [
     'list = copr_cli.subcommands:List',
     'add-copr = copr_cli.subcommands:AddCopr',
+    'build-copr = copr_cli.subcommands:Build',
 ]
 
 __name__ = 'copr-cli'
-- 
1.8.1