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

Re: query a specific cartridge by id




----- Original Message -----
> 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?

Given the choice between /cartridge?name=<blah> and /cartridge/<blah> the latter is more rails like and more correct, and also plays better with other problems.  With my changes you should be able to do /cartridge?name=<blah> and it might work - right now the parameter names are wrong for that.  Either way, parameters aren't everything.

> 
> 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?

We'll need to ensure that new clients against old jenkins servers don't fail, but I don't worry about that.

> 
> >
> >> 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]