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

Re: querying non-existing cartridge is not erroring?



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


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