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

Re: query a specific cartridge by id



Hi Andre and all,
We can take one of the below approaches.

1.  Return the obsolete cartridges for older API versions.  I believe the java client uses API 1.2?

2.  Always return the obsolete cartridges.  The clients rhc, console, etc will have to be modified to either filter these out or at least show that they are obsoleted.

I'll let people weight in with their opinions and take it from there.

Lili


----- Original Message -----
From: "André Dietisheim" <adietish redhat com>
To: "Lili Nader" <lnader redhat com>, "Clayton Coleman" <ccoleman redhat com>
Cc: dev lists openshift redhat com
Sent: Friday, January 31, 2014 6:28:27 AM
Subject: Re: query a specific cartridge by id

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]