[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: query a specific cartridge by id



On 01/30/2014 05:19 PM, Clayton Coleman wrote:

----- Original Message -----
Hi Claytong, hi Lili

I'm trying to understand things here. so

1) If I get it right :name is a variable that should get subsituted in
the url (href in the REST response). I was wondering why we'd do url
building here, I was assuming that links in REST responses should always
be used as and point to REST resources and and thus url-building was a
bad thing in REST. What am I missing?
Depends.  If there are 300 cartridges and you can't keep a copy of that list, do you want to do the following every time?

   /cartridges -> 120k, 40ms
   <follow link> -> 10k, 10ms

In a web UI that's 50ms and 130k of download to get what really should be 10k and 10ms.  The link relation is a contract between client and server - the point is that clients don't build links themselves so you can change those links in the future.  Nothing stops someone from continuing to link build - but after all, when you add the request parameter onto one of our existing links, you're really building a URL.

Sure fully get that and that's also the case where one wants to search/filter/paginate large lists. Afaik a typical solution to this is to use parameters. Can you explain to me why you guys chose to let the client build the url (via substitution) vs. provide the variable parts in parameters?

I'll go for parameter substituion since there's obviously no way around it and I personally prefer the single result I get in cartridge/:name vs the mutli-entry result-set I get with cartridges?name= which makes me filter/have logic in the client that returns a single cartridge (see current PR).

As far as I understood the changes in backend required by the jenkins-plugin will be available in the end of this sprint. I thus guess that I can postpone this until the changes get merged in, right?


2) providing "name" via url-parameter is equivalent to appending name in
the url?
Yes, in my changes going in this sprint.

My current guess is that they are not since I found the following href
in the API resource:
"https://openshift.redhat.com/broker/rest/domain/:domain_name/application/:name";
which seems to impose var substitution, right?
Yes

I also see ?id= and /:id not being equivalent in the following scenarios:
* GET'ing /cartridge/:id with an invalid id (ex. bogus) returns an empty
response, if I read you right /cartridge/:id will return 404 once your
patch is merged, right?
Yes, and i'm going to change the link so that it's :name and there's a separate link for :id.  Both scenarios are relevant now.

* GET'ing /cartridge/zend returns a single resource, the earlier
zend-5.6, zend-6.1 is not returned. GET'ing /cartridge?id=zend returns
both zend cartridges.
So this looks like var substitution is a thing we'll have to suppport in
the client, right?
If you want to lookup a single cart by name, and get it via that /cartridge/ url, yes.

3) Do you have some doc that explains the "standard query string
interpolation API syntax" you're referring to? Or please point me to
some (ruby?) code that helps me get the rules?
If there is a parameter name that starts with ':', the link requires interpolation.  So instead of creating a request parameter, do:

   get link path
   replace all instances of :param with the URL encoded (this part is important) value
   continue as before for other parameters

See https://github.com/openshift/rhc/blob/master/lib/rhc/rest/base.rb#L25 for an example



On 01/30/2014 12:50 AM, Clayton Coleman wrote:
I've got the "by id" loading in my fork on top of lilis changes - when that
drops all cartridges report an "id" that uniquely represents that version
of the cartridge in use, which you can pass to application create.  In the
event id is not available, you can pass name/ to cartridge/:id by
following our standard query string interpolation API syntax - see
show_application with the :id syntax.

On Jan 29, 2014, at 3:50 PM, Lili Nader <lnader redhat com> wrote:

Andre and all,
Since the openshift java client does not have a way for constructing
/cartridge/id URL (I'm referring to our previous discussion over email) I
implemented the /cartridges?id=<name> which will return an empty data:[]
if the cartridge is not found.  The correct way to get a single cartridge
is /cartridge/id, which would return a 404 if the cartridge does not
exists.

Since we have to support the old API which makes queries
/cartridges/standalone and /cartridges/embedded I also put in a fix to
route the request to index.  See

https://github.com/lnader/origin-server/commit/3912b67d681cb0d3872121992363d375ea29a57b

Note: This change has not yet been pushed to openshift.redhat.com.

Lili

----- Original Message -----
From: "Clayton Coleman" <ccoleman redhat com>
To: "Ben Parees" <bparees redhat com>
Cc: dev lists openshift redhat com
Sent: Wednesday, January 29, 2014 9:11:33 AM
Subject: Re: querying non-existing cartridge is not erroring?

So we'll want to do name to start, but the type's unique if will get
exposed and you'll be able to retrieve via id.  The by id should land
this sprint.

On Jan 29, 2014, at 11:58 AM, Ben Parees <bparees redhat com> wrote:

So are we going to expect the builder to use the exact older version,
then?  Currently it just gets the cartridge by name...


Ben Parees | OpenShift

----- Original Message -----
From: "Clayton Coleman" <ccoleman redhat com>
To: "Ben Parees" <bparees redhat com>
Cc: "André Dietisheim" <adietish redhat com>,
dev lists openshift redhat com
Sent: Wednesday, January 29, 2014 11:48:07 AM
Subject: Re: querying non-existing cartridge is not erroring?

My changes to cartridges are going to stomp all over this assumption.
Going
to have to give you a fix in card #193 that makes this work.

However from an API perspective we should be using /cartridge/:name and
we
should allow you to fetch an inactive (obsolete is changing) cartridge
by
name (there may be hundreds of older versions).

On Jan 29, 2014, at 11:42 AM, Ben Parees <bparees redhat com> wrote:

This PR is to address the problem that jenkins can't currently get
obsolete
cartridges for purposes of creating a builder in the case where an
existing app is running on a cartridge that is now obsolete.



Ben Parees | OpenShift

----- Original Message -----
From: "André Dietisheim" <adietish redhat com>
To: "Clayton Coleman" <ccoleman redhat com>
Cc: dev lists openshift redhat com
Sent: Wednesday, January 29, 2014 11:06:39 AM
Subject: Re: querying non-existing cartridge is not erroring?

On 01/29/2014 05:03 PM, Clayton Coleman wrote:
Let me answer with a question - what do you need the data for
obsolete
cartridges for?  If it's for apps that already have that cart I would
recommend using the app cartridges list.
I have no clue about the value of deprecated cartridges. I got a PR on
the openshift-java-client where I was told that the reasoning for it
was
to be able to query deprecated cartridges that would not show up in
the
list of available cartridges:
https://github.com/openshift/openshift-java-client/pull/109

If I misunderstood the reasoning then I'd love to know what the reason
for adding this is?
The client lib tries to avoid unnecessary queries and caches the list
of
available cartridges. If this PR is only about getting a cartridge by
name then I'd change the suggested change to query the cached list
instead of querying the backend.

On Jan 29, 2014, at 10:43 AM, André Dietisheim <adietish redhat com>
wrote:

I'd like to verify an assumption which I'm not sure is right:

The curl (/cartridges?id=<cartname>) is as far as I can understand
querying cartridges by name. I was assuming that deprecated
cartridges
(I
was told that you'll deprecate carts in the future) could be queried
in
this way while they would not show up in /cartridges.
Is this correct?

On 01/29/2014 04:15 PM, Clayton Coleman wrote:

On Jan 29, 2014, at 10:13 AM, André Dietisheim
<adietish redhat com>
wrote:

On 01/29/2014 04:09 PM, Clayton Coleman wrote:
We shouldn't change legacy behavior though - cartridges/:id
should
behave like old code, but cartridge/:id should behave like new,
and
if
we need a search behavior we should use a different attribute
(like
name, since carts don't have public ids today).  Did
cartridges/:id
change for old clients in a breaking way?
for openshift-java-client there's no breakage since this was not
implemented before (I got this via a new patch I was verifying and
writing tests for).
We really shouldn't change that old behavior - I have some changes
landing soon that restore the old behavior and add ?name= prefix
search
under /cartridges.

On Jan 29, 2014, at 10:01 AM, Jordan Liggitt
<jliggitt redhat com>
wrote:

Typically, when using a filter which can return multiple results
(e.g.
https://openshift.redhat.com/broker/rest/cartridges.json?id=nodejs),
an empty result set is expected rather than a 404.




On 01/29/2014 09:51 AM, André Dietisheim wrote:
Hi LIli, hi all

I was writing tests for the new query to get infos about a
cartridge.
I stumbled upon the following: When I do

curl -H "Accept: application/json; version=1.2" --user
adietish redhat com:1q5T9o3E$
https://openshift.redhat.com/broker/rest/cartridges?id=bogus -X
GET
| json_reformat

I dont get 404 as I had expected but the following:

{
   "api_version": 1.2,
   "data": [

   ],
   "messages": [
       {
           "exit_code": 0,
           "field": null,
           "index": null,
           "severity": "info",
           "text": "List bogus cartridges"
       }
   ],
   "status": "ok",
   "supported_api_versions": [
       1.0,
       1.1,
       1.2,
       1.3,
       1.4,
       1.5,
       1.6
   ],
   "type": "cartridges",
   "version": "1.2"
}

Is this expected or a bug as I tend to think? If this is a bug,
was
this filed or should I?

Thanks
André

_______________________________________________
dev mailing list
dev lists openshift redhat com
http://lists.openshift.redhat.com/openshiftmm/listinfo/dev
_______________________________________________
dev mailing list
dev lists openshift redhat com
http://lists.openshift.redhat.com/openshiftmm/listinfo/dev
_______________________________________________
dev mailing list
dev lists openshift redhat com
http://lists.openshift.redhat.com/openshiftmm/listinfo/dev
_______________________________________________
dev mailing list
dev lists openshift redhat com
http://lists.openshift.redhat.com/openshiftmm/listinfo/dev
_______________________________________________
dev mailing list
dev lists openshift redhat com
http://lists.openshift.redhat.com/openshiftmm/listinfo/dev



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]