[PATCH 3/3] Create the API namespace

Pierre-Yves Chibon pingou at pingoured.fr
Wed Jan 23 12:24:34 UTC 2013


On Wed, 2013-01-23 at 07:11 -0500, Bohuslav Kabrda wrote:
> ----- Original Message -----
> > This is the namespace that we will use to describe and expose the
> > copr API.
> > This commit introduce it, move some code from misc to the api_general
> > and
> > start with two API functions to list someone's copr and add a new
> > one.
> > 
> > These two functions are documented in the /api/ (ie in the api.html
> > template).
> > ---
> > 
> > This is very simple for the moment and I a beginning of a CLI that
> > can call this
> > API and do something with it. That will be for a later commit.
> > 
> > One point though, I was not able to allow sending the data via post,
> > I am afraid
> > that this is due to the fact that I do not use forms. I might be
> > better finally
> > to just re-use the forms with csrf deactivated. Bonus, re-using the
> > forms gives
> > us input validation as well.
> > 
> > Thoughts on this?
> > 
> 
> Could you be more elaborate on what the problem with post data was?
> Reusing forms seems to be a good idea, although we have to be careful to pass the data in the format that forms expect.
> 
> <snip>
> > + at api_ns.route('/add/<name>/<chroots>/',
> > +    defaults={'repos':"", 'initial_pkgs':""})
> > + at api_ns.route('/add/<name>/<chroots>/<repos>/',
> > +    defaults={'initial_pkgs':None})
> > + at api_ns.route('/add/<name>/<chroots>/<repos>/<initial_pkgs>/')
> > + at login_required
> > +def api_add_copr(name, chroots, repos="", initial_pkgs=""):
> </snip>
> 
> Hmm, I really wouldn't do this. Let's take the time to do it via POST or GET params, not part of the url.

You have here the answer to the question above. We need to re-use the
form or parse the request argument by hand (:-/) to be able to use POST
and/or GET.

> <snip>
> > + at api_ns.route('/list/<username>/', methods=['GET'])
> > +def api_list_copr(username):
> </snip>
> 
> What I'd like to see here is copying the URLs from the web frontend part. E.g. listing all coprs should be just '/api/coprs/', listing coprs for a user '/coprs/owned/<username>/'. Also, I'd name the action for creation "new", not "add", as it corresponds with "copr_new" from coprs_ns, not "copr_add".

I am afraid we are going to generate a lot of code duplication.
I wonder if we couldn't make some sort of global variable which would
allow to distinguish website/cli.
Something like:

flask.g.output = 'html'

# In the login method:
if call relies on api_token:
   flask.g.output = 'json'

# In the views:
if output == 'json':
  return convert_to_json...
else:
  return render_template()

No idea if that would work but it would clearly reduce code duplication
(although it might also make keeping the API stable harder).

Pierre


More information about the copr-devel mailing list