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

Re: query a specific cartridge by id



I agree - I'll make sure that tweak goes into my changes (which should land today)

> On Jan 31, 2014, at 9:28 AM, André Dietisheim <adietish redhat com> wrote:
> 
> Hi Lili, Hi Clayton
> 
> Looking into details I see existing JBDS users being affected by the current approach for deprecating cartridges.  Existing users would not see the type of existing applications any longer (in the import/new app-wizard) since we match the existing cartridge against the list of available cartridges. There are eventually other pain-points that we would have to consider.
> A far less intrusive approach would be to still list but mark deprecated cartridges. Newer client would be able to filter these out, existing users would not get broken.
> 
> Cheers
> André
> 
> 
>> On 01/31/2014 11:32 AM, André Dietisheim wrote:
>> Hi Lili
>> 
>> thanks for the nice summary, helps!
>> So if I get it right both /cartridge/:id and /cartridges?id= would return deprecated carts (whereas /cartridges/ would not).
>> 
>>> On 01/30/2014 09:48 PM, Lili Nader wrote:
>>> I'm not in the loop on id versus name change that Clayon is putting in place.  Clayton, can you please elaborate on the difference between id and name moving forward?
>>> 
>>> Not including Clayton's pending changes this is the expected behaviour:
>>> 
>>> /cartridges - returns a list of cartridges (Only includes "available" cartridges. i.e. not obsolete)
>>> /cartridge/id - returns a single cartridge or 404 (NOTE: the url is /cartridge (singular) and not /cartridges (plural))
>> 
>> What I dont fully understand here yet is that i.ex. /cartridges/zend would return only the earlier zend (there are 2 zends, 6.1 and 5.6, I only get 5.6). In Node on the other hand, I always get the latest nodejs-0.10 when I query /cartridge/nodejs.
>> 
>>> 
>>> The exceptions to the above rule for backward compatibility:
>>> 
>>> /cartridges/embedded - returns a list of embedded cartridges
>>> /cartridges/standalone - returns a list of standalone cartridges
>>> 
>>> The below is meant to be a search/filter
>>> 
>>> /cartridges?id=<name> - returns a list of cartridges matching the name. i.e. like a search
>>> 
>>> but I agree that it should be more clear like
>>> 
>>> /cartridges?search=<search-string> - returns a list of cartridges matching the search
>> 
>> I have the impression that cartridges?name= would better express that one has to provide the cartridge-name
>> 
>>> 
>>> The following link (returned in /api response) should give you all the information you need to construct the URL. i.e. You need to substitute :id in the link's href.  All SHOW_<RESOURCE> links follow this format.  So yes clients (if they want to use the the SHOW links) have to do some URL building but the format is outlined for them.
>>> 
>>> "SHOW_CARTRIDGE": {
>>>             "href": "https://localhost/broker/rest/cartridge/:id";,
>>>             "method": "GET",
>>>             "optional_params": [],
>>>             "rel": "Retrieve a cartridge by name",
>>>             "required_params": [
>>>                 {
>>>                     "description": "Name of the cartridge",
>>>                     "invalid_options": [],
>>>                     "name": ":id",
>>>                     "type": "string",
>>>                     "valid_options": []
>>>                 }
>>>             ]
>>>         },
>>> 
>>> 
>>> 
>>> Lili
>>> 
>>> 
>>> 
>>> 
>>> 
>>> ----- Original Message -----
>>> From: "Clayton Coleman" <ccoleman redhat com>
>>> To: "André Dietisheim" <adietish redhat com>
>>> Cc: "Lili Nader" <lnader redhat com>, dev lists openshift redhat com
>>> Sent: Thursday, January 30, 2014 10:55:41 AM
>>> Subject: 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]