*** pratikmallya has joined #senlin | 00:56 | |
*** zhenguo has joined #senlin | 01:21 | |
*** Yanyanhu has joined #senlin | 01:28 | |
*** rebase_ has quit IRC | 01:28 | |
*** lixinhui_ has joined #senlin | 01:39 | |
*** Qiming has joined #senlin | 02:03 | |
xuhaiwei | Qiming, are you woring on the http response fix bp now? | 02:05 |
---|---|---|
xuhaiwei | the second step | 02:06 |
Qiming | yes, xuhaiwei | 02:06 |
Qiming | as for the api-site patch | 02:06 |
xuhaiwei | the patch is on the way? | 02:06 |
Qiming | it is just a first step to add api docs | 02:06 |
Qiming | we have a long way to complete the bp | 02:06 |
Qiming | but we are not blocking the api docs because we have plans to revise them | 02:07 |
xuhaiwei | I mean if you are not writing patch about how to return status code to client, I will write one | 02:07 |
xuhaiwei | can we do it now? | 02:07 |
Qiming | that will be good | 02:07 |
xuhaiwei | ok | 02:07 |
Qiming | we will have per-api patch proposed and reviewed | 02:08 |
xuhaiwei | I reviewed the patch you sent to api-ref | 02:08 |
Qiming | "The http response status code is quite different from what we have fixed in Senlin" | 02:08 |
Qiming | this is incorrect | 02:09 |
xuhaiwei | that is your intension? | 02:09 |
Qiming | it is creating an impression to the api project that the doc is not mature | 02:09 |
Qiming | there is nothing we HAVE FIXED so far | 02:09 |
Qiming | we added 'success' to be an intended status code to be returned | 02:10 |
Qiming | it is not yet effected | 02:10 |
xuhaiwei | I know that, so the api-ref just keep the current status code, and you will fix it later again? | 02:10 |
Qiming | also, if you are reading the wadl file carefully, profile_create.json is the request body, profile_create_resp.json is a response | 02:11 |
Qiming | if we are saying that we return 201 for profile_create today | 02:11 |
Qiming | are we telling the truth? | 02:11 |
xuhaiwei | I think the api-ref should tell the right thing, that means we need to fix them in senlin | 02:12 |
Qiming | as for the response json files' names, is there any requirement to have them named the same? | 02:12 |
xuhaiwei | write something wrong and then fix them? | 02:12 |
Qiming | it is not something wrong | 02:12 |
Qiming | nothing is wrong, man | 02:12 |
Qiming | we are improving things | 02:13 |
Qiming | when we have fixed the code, we revise the API doc to reflect the API change | 02:13 |
xuhaiwei | so why not improve them in one time | 02:13 |
Qiming | that will be a step-by-step way | 02:13 |
Qiming | an API document is supposed to reflect the current code implementation, right? | 02:13 |
xuhaiwei | or fix the code first, and then push the api doc | 02:14 |
Qiming | it is not a design, not a vision | 02:14 |
Qiming | the goal is to have them consistent | 02:14 |
Qiming | if we wait for all API changes to land and then propose the API doc | 02:15 |
Qiming | can you estimate when we can publish an API doc? | 02:15 |
xuhaiwei | I also saw some of the status code like DELETE action is returning 204, that's not what we are doing now | 02:15 |
Qiming | if you are seeing things inconsistent, file a bug and fix it | 02:15 |
xuhaiwei | I mean in the api-ref | 02:16 |
Qiming | I understand | 02:16 |
Yanyanhu | hi, Qiming, xuhaiwei, IMHO, I think it's right to give an API doc which reflects the REAL status of our CURRENT API implementation | 02:16 |
xuhaiwei | ok | 02:16 |
Yanyanhu | just feel mark it as OpenStack Clustering API v1 could have some problems | 02:17 |
Yanyanhu | since that means if we change the API interface later, we have to bump to v2 | 02:17 |
Qiming | we are still working on it | 02:17 |
Qiming | we are not doing a formal release yet | 02:17 |
Qiming | that means we are supposed to work harder on getting things fixed, improved | 02:17 |
Qiming | we are already missing mitaka-1 release today | 02:18 |
xuhaiwei | about the json file names, why not keep them consistent? | 02:18 |
Qiming | they are just samples, referenced from the wadl | 02:18 |
Qiming | keeping them consistent would be good | 02:18 |
Qiming | but I don't see it a bug if we have both xxx_resp.json and xxx_response.json, provided we are getting the links right | 02:19 |
Yanyanhu | ok, so the later change to API doc will be treated as fix, not new API design | 02:19 |
Qiming | yes, those are all fixes | 02:19 |
xuhaiwei | yes, that's some nits | 02:19 |
Qiming | if you have been working on fixing api docs previously, you will feel differently | 02:20 |
Yanyanhu | ok, if so, that makes sense. | 02:20 |
*** elynn has joined #senlin | 02:20 | |
xuhaiwei | about other json files like cluster_create.json, they are request body? | 02:20 |
Qiming | xuhaiwei, check the wadl file and see where they are linked from | 02:21 |
Qiming | it is refrenced from line 290 | 02:21 |
Qiming | it IS are request body | 02:21 |
Qiming | line 339 is a link to the sample response | 02:22 |
xuhaiwei | I think I need to learn how to read the file | 02:24 |
Qiming | yes, indeed | 02:24 |
Qiming | or you can start working on fixing the code | 02:25 |
openstackgerrit | Yanyan Hu proposed openstack/senlin: Give unique name to all resources created in functional test https://review.openstack.org/252713 | 02:26 |
xuhaiwei | ok | 02:27 |
Qiming | there are higher priority things we need to fix | 02:28 |
Qiming | for example: https://review.openstack.org/#/c/251204/ | 02:28 |
Qiming | we are not returning an object | 02:28 |
Qiming | this is bad | 02:28 |
Qiming | need fix both the code and the doc | 02:28 |
Qiming | but if we wait for all these patches to land before proposing the api doc | 02:28 |
Qiming | it will take forever | 02:29 |
Qiming | the only thing that never changes is change | 02:29 |
Qiming | lifeless was putting that right, see https://review.openstack.org/#/c/226157/11/specs/backwards-compat-libs-clients.rst | 02:30 |
Qiming | line 47-52 | 02:30 |
*** rebase_ has joined #senlin | 02:32 | |
*** rebase_ has quit IRC | 02:35 | |
xuhaiwei | maybe we need to do version management like Nova | 02:36 |
xuhaiwei | in the future | 02:36 |
Qiming | yes, we will | 02:37 |
Qiming | the first thing first is get the V1 fixed | 02:37 |
Qiming | make it as good as we can | 02:37 |
xuhaiwei | ok | 02:37 |
Qiming | before V1 is stabilized, we are not working on version or micro-version management | 02:38 |
Qiming | before we have version management, we are not bumping the API version | 02:38 |
xuhaiwei | yes, it is not necessary now | 02:38 |
Yanyanhu | agree that fixing v1 is the most important thing for now | 02:39 |
Qiming | we are not confident to invite people to try this thing if we know for sure the API will change a lot | 02:40 |
Yanyanhu | yes, definitely | 02:40 |
Qiming | what concerns me now is the sdk side support to 201 | 02:40 |
Qiming | and 202 | 02:40 |
Qiming | we are gonna have a lot of troubles there | 02:40 |
Qiming | we need hands to work on them | 02:40 |
Yanyanhu | what is sdk team's opinion about this issue? | 02:41 |
Yanyanhu | they want to fix it but just lack of resource? | 02:41 |
Qiming | we are still hosting our own models at client side: http://git.openstack.org/cgit/openstack/python-senlinclient/tree/senlinclient/v1/models.py | 02:41 |
Qiming | all those things are to be removed | 02:41 |
Yanyanhu | yes, this is a problem | 02:41 |
Qiming | I'm busy like a dog | 02:41 |
Yanyanhu | understand... | 02:42 |
lixinhui_ | everyone like dog | 02:42 |
Qiming | all lines after this: http://git.openstack.org/cgit/openstack/python-senlinclient/tree/senlinclient/v1/client.py#n268 | 02:42 |
Qiming | should be removed | 02:42 |
Yanyanhu | they should become proxy call? | 02:43 |
Yanyanhu | function call | 02:43 |
xuhaiwei | currently which one is not landed in SDK? | 02:43 |
xuhaiwei | or all of them? | 02:43 |
Qiming | https://review.openstack.org/#/c/250574/ | 02:43 |
Qiming | https://review.openstack.org/#/c/251204/ | 02:44 |
Qiming | we still need to add cluster_policy model | 02:44 |
Qiming | a revised version of webhook | 02:44 |
Qiming | profile_type | 02:45 |
Qiming | policy_type | 02:45 |
Qiming | cluster_node | 02:45 |
Qiming | when all these landed, we can delete all lines after this: http://git.openstack.org/cgit/openstack/python-senlinclient/tree/senlinclient/v1/client.py#n32 | 02:46 |
xuhaiwei | these jobs are not related to the API fix I think | 02:47 |
Qiming | ... | 02:48 |
xuhaiwei | they are also in very high priority? | 02:48 |
xuhaiwei | don't cry sir | 02:49 |
Qiming | without getting APIs stabilized | 02:49 |
Qiming | we are not proposing a stable resource to SDK | 02:49 |
Qiming | withou SDK side landing of senlin resources | 02:49 |
Qiming | we won't be able to cleanse the senlinclient thing | 02:50 |
xuhaiwei | so still the api side should be more urgent? | 02:50 |
Qiming | yes, it is top priority | 02:51 |
xuhaiwei | ok, let's do it | 02:51 |
Qiming | make the code work as expected | 02:51 |
Qiming | make the document published and reflecting the status quo of code implementation | 02:51 |
Qiming | announce in the community: we are releasing our first official version 1.0.0, it is stable | 02:52 |
Qiming | then we catch up with the community by tagging senlin as release: milestones_with_intermediaries | 02:53 |
xuhaiwei | still a long way to go | 02:55 |
Qiming | yes, xuhaiwei | 02:56 |
Qiming | so I'm gonna ignore your complaints about the naming of xxx_resp.json or xxx_response.json | 02:56 |
xuhaiwei | ok | 02:56 |
Qiming | feel free to file a bug, will jump onto it later | 02:56 |
openstackgerrit | Yanyan Hu proposed openstack/senlin: Merge cluster/node list test to other test cases https://review.openstack.org/252724 | 03:09 |
*** pratikmallya has quit IRC | 03:15 | |
openstackgerrit | Merged openstack/senlin: Give unique name to all resources created in functional test https://review.openstack.org/252713 | 03:15 |
*** yuanying_ has joined #senlin | 03:18 | |
openstackgerrit | Qiming Teng proposed openstack/senlin: Make build-info api return an object https://review.openstack.org/252725 | 03:19 |
*** yuanying has quit IRC | 03:20 | |
*** yuanying_ has quit IRC | 03:33 | |
*** rebase_ has joined #senlin | 03:36 | |
*** rebase_ has quit IRC | 03:37 | |
*** yuanying has joined #senlin | 03:38 | |
*** pratikmallya has joined #senlin | 04:01 | |
*** yuanying has quit IRC | 04:05 | |
*** yuanying has joined #senlin | 04:08 | |
*** elynn has quit IRC | 04:15 | |
*** pratikmallya has quit IRC | 04:23 | |
*** elynn has joined #senlin | 04:48 | |
openstackgerrit | xu-haiwei proposed openstack/senlin: Make senlin API return correct status code https://review.openstack.org/252741 | 04:56 |
openstackgerrit | Merged openstack/senlin: Make build-info api return an object https://review.openstack.org/252725 | 05:46 |
Qiming | publisher job has been merged | 05:52 |
Yanyanhu | cool! | 05:52 |
elynn | cool! | 05:52 |
Qiming | still waiting for doc to show up on docs.openstack.org/developer/senlin/ | 05:52 |
openstackgerrit | Merged openstack/senlin: Make senlin API return correct status code https://review.openstack.org/252741 | 05:55 |
Qiming | okay, now the API is returning the correct status code, have to rework the api doc | 06:08 |
Yanyanhu | hi, xuhaiwei, I found api resp code doesn't change after applying this patch? | 06:08 |
Yanyanhu | https://review.openstack.org/252741 | 06:08 |
Yanyanhu | Qiming, so it works well in you env? | 06:08 |
Qiming | that is a different thing | 06:09 |
xuhaiwei | due to my test, it is changed | 06:09 |
Yanyanhu | ok, I guess something wrong in my env | 06:09 |
Qiming | we are returning the expected status code now | 06:09 |
Qiming | something will break | 06:09 |
Qiming | but the api doc needs a revision | 06:09 |
Yanyanhu | I just pull the latest code and reinstall senlin service | 06:09 |
Yanyanhu | Qiming, yes | 06:09 |
Qiming | if things are broken, it is not the api doc's problem | 06:09 |
Yanyanhu | Qiming, time to change API doc now | 06:09 |
xuhaiwei | for example you create a cluster, the response code is not 202? | 06:10 |
Qiming | \o/ | 06:10 |
Qiming | http://docs.openstack.org/developer/senlin/ | 06:10 |
Qiming | it IS there !!! | 06:10 |
Yanyanhu | nice! | 06:10 |
Qiming | welcome to earth, senlin docs! | 06:10 |
xuhaiwei | here we come! | 06:10 |
Yanyanhu | haha | 06:10 |
Qiming | enjoy it!!! | 06:11 |
Yanyanhu | :) | 06:11 |
Qiming | guys, I'm dancing | 06:12 |
Qiming | I'm not crazy, don't worry | 06:12 |
xuhaiwei | I think that patch is not merged yet, yanyan | 06:12 |
Yanyanhu | :P | 06:12 |
xuhaiwei | merged but not landed | 06:12 |
Yanyanhu | what you mean by 'not landed'? | 06:13 |
xuhaiwei | not into the master branch yet | 06:13 |
xuhaiwei | can you check the source? | 06:13 |
Yanyanhu | yes, I saw it in my local code | 06:13 |
Yanyanhu | I see the change | 06:13 |
Yanyanhu | about use 'success' code as resp status_code | 06:14 |
xuhaiwei | it still doesn't work? | 06:14 |
Yanyanhu | just don't know why it doesn't take effect | 06:14 |
Yanyanhu | yes... | 06:14 |
Qiming | after this is merged, we will have links from the index.html : https://review.openstack.org/#/c/252374/ | 06:14 |
Yanyanhu | I cleaned all *.pyc and reintall service | 06:14 |
Qiming | Yanyanhu, what do you mean by it doesn't work | 06:15 |
Qiming | what doesn't work | 06:15 |
xuhaiwei | the response code is not changed | 06:15 |
xuhaiwei | he means | 06:15 |
xuhaiwei | DEBUG (session) REQ: curl -g -i -X POST http://192.168.11.81:8778/v1/d59cb05324954496b86475cc9afbbd65/clusters -H "User-Agent: python-senlinclient" -H "Content-Type: application/json" -H "X-Auth-Token: {SHA1}b896315a6d8e55aa080a1b13e6e612c9b293cf44" -d '{"cluster": {"name": "cluster2", "parent": null, "profile_id": "pro_nova", "min_size": 0, "desired_capacity": 0, "timeout": null, "max_size": -1, "metadata": {}}}' INFO (connectionp | 06:15 |
Yanyanhu | yes | 06:15 |
Qiming | Yanyanhu, when you do cluster-create | 06:15 |
xuhaiwei | it works for me | 06:15 |
Yanyanhu | I think something error happened in my env | 06:15 |
Qiming | you will get a 202 | 06:15 |
Qiming | then the client is doing another get | 06:15 |
Qiming | which returns 200 | 06:15 |
Qiming | check log output from 'senlin -d cluster-create' carefully | 06:16 |
xuhaiwei | the 200 is GET method which gets the new cluster | 06:16 |
Yanyanhu | yes, but when I used --debug mode to test delete, I saw 204 rather than 202 is returned to me | 06:16 |
Yanyanhu | weird | 06:17 |
Qiming | sigh | 06:17 |
Qiming | it is not weird I think | 06:17 |
xuhaiwei | some DELETE returns 204 | 06:17 |
Yanyanhu | yes, but I think cluster-delete should give 202 as resp code | 06:17 |
Qiming | We are not cleansing all 'raise exc.HTTPNoContent' | 06:17 |
Qiming | read the source | 06:18 |
Yanyanhu | oh | 06:18 |
Yanyanhu | I see | 06:18 |
Yanyanhu | let me test create | 06:18 |
Qiming | I believe there are a lot of other inconsistency | 06:18 |
xuhaiwei | I will start the cleaning job | 06:18 |
Qiming | thanks | 06:18 |
Qiming | xuhaiwei, please do this slowly, one by one | 06:19 |
xuhaiwei | ok, first to clean UPDATE method | 06:19 |
Qiming | for example, when an api is supposed to return a 202 | 06:19 |
Qiming | we need to add a comment (TODO) there, because we are not returning the proper 'Location' header | 06:20 |
Qiming | it is a partial fix | 06:20 |
xuhaiwei | ok | 06:20 |
Qiming | also please help do some research on 202, whether we are supposed to return something in the body, or we are supposed to return nothing in body | 06:21 |
Qiming | cannot recall exactly what the api-wg suggests | 06:21 |
Qiming | iirc, api-wg is not very clear about this | 06:21 |
Qiming | let's find out what is the best practice | 06:21 |
xuhaiwei | I will check api-wg manual again | 06:22 |
Yanyanhu | will propose a change in functional test since part API interfaces have changed its response status_code | 06:22 |
Qiming | I'm focusing on making the api doc consistent | 06:22 |
Qiming | cool | 06:22 |
Yanyanhu | and haiwei, if you can also revised the functional test each time you remove a "raise" statement in API implementation, that would be best :) | 06:23 |
Yanyanhu | I will try to build a correct based base on this patch https://review.openstack.org/252741 | 06:23 |
Yanyanhu | since functional still doesn't work correctly for some DB sync issues I think | 06:24 |
Yanyanhu | I'm trying to avoid making it more messy... | 06:24 |
xuhaiwei | sorry to bring you trouble Yanyanhu, I will do that | 06:27 |
xuhaiwei | thought I dont know function test very well | 06:27 |
Yanyanhu | no problem :) I will try to make a revision to make it match current status | 06:28 |
*** pratikmallya has joined #senlin | 07:05 | |
openstackgerrit | xu-haiwei proposed openstack/senlin: Remove HTTPAccept exception raised when update cluster https://review.openstack.org/252786 | 07:06 |
Qiming | hey, guys | 07:11 |
Qiming | when I'm revising the api doc | 07:11 |
Qiming | I am thinking if we should remove "created_time", "updated_time" and "deleted_time" from the filters for listing | 07:12 |
Qiming | they are pretty difficult to input | 07:12 |
openstackgerrit | xu-haiwei proposed openstack/senlin: Remove HTTPAccept exception from node update API https://review.openstack.org/252787 | 07:13 |
xuhaiwei | some time 'created_time' is usefull | 07:13 |
Yanyanhu | hmm, I think we should since people could use timestamp to do sorting but not filtering I think | 07:13 |
Qiming | it can be used to do sorting | 07:13 |
xuhaiwei | when clusters or nodes are too many, people may want to know who is newer | 07:14 |
Qiming | to make 'created_time' based filtering work, basically, you will need to know the exact second when the object was created | 07:14 |
Qiming | I'm not proposing we remove them from the object properties | 07:14 |
Qiming | I'm concerning about use timestamp or time string to do filtering | 07:15 |
xuhaiwei | you mean there maybe some resources which have the exact same timestamp??? | 07:15 |
Qiming | ... | 07:16 |
xuhaiwei | are you worried about that? | 07:16 |
xuhaiwei | sometime that happens, because the timestamp is not so exact | 07:16 |
Qiming | I'm concerning how useful it would be | 07:16 |
xuhaiwei | you mean if people want to know who is older they can check the create_time by themselves? | 07:17 |
Qiming | will you do a cluster list like this? senlin cluster-list -f "created_time=2015-11-23T08:33:13" | 07:17 |
xuhaiwei | I got you | 07:18 |
Qiming | my question " if we should remove "created_time", "updated_time" and "deleted_time" from the filters for listing" | 07:18 |
xuhaiwei | that's unuseful | 07:18 |
Qiming | okay | 07:19 |
Qiming | then we should remove line 72-74 from http://git.openstack.org/cgit/openstack/senlin/tree/senlin/api/openstack/v1/profiles.py | 07:19 |
xuhaiwei | I am ok to fail that function | 07:19 |
Yanyanhu | yes, we should | 07:20 |
Qiming | if we SHOULD remove them | 07:21 |
Qiming | then NOW is the right time | 07:21 |
Qiming | and the same applies to policies | 07:22 |
xuhaiwei | ok | 07:24 |
openstackgerrit | xu-haiwei proposed openstack/senlin: Remove HTTPAccept exception from node update API https://review.openstack.org/252787 | 07:24 |
Qiming | in future, we can support something like "get me all the profiles created before last Monday" | 07:25 |
Qiming | that will be some query string like "/.../profiles?created_time=lt:2015-11-01" | 07:26 |
Qiming | like proposed here: http://git.openstack.org/cgit/openstack/api-wg/tree/guidelines/pagination_filter_sort.rst#n94 | 07:26 |
Qiming | anyway, exact matching of timestamps are useless | 07:27 |
Qiming | need to remove them | 07:27 |
openstackgerrit | lvdongbing proposed openstack/python-senlinclient: Fix nit in the help msg of cluster-node-del https://review.openstack.org/252794 | 07:29 |
openstackgerrit | Merged openstack/python-senlinclient: Fix nit in the help msg of cluster-node-del https://review.openstack.org/252794 | 07:45 |
openstackgerrit | xu-haiwei proposed openstack/senlin: Remove HTTPAccept exception raised when update cluster https://review.openstack.org/252786 | 07:50 |
xuhaiwei | Qiming | 07:53 |
xuhaiwei | according to the api-wg, only creation action return a 'location' header | 07:53 |
openstackgerrit | Yanyan Hu proposed openstack/senlin: Change action.start_time and action.end_time data type to REAL https://review.openstack.org/252803 | 07:54 |
xuhaiwei | https://github.com/openstack/api-wg/blob/master/guidelines/http.rst#2xx-success-codes | 07:54 |
Qiming | xuhaiwei, I don't think that is true | 07:54 |
Qiming | my understanding is different | 07:54 |
xuhaiwei | in fact I don't know why the location header should be included | 07:55 |
Qiming | it only says that a create action should contain a 'location' header if 202 is to be returned | 07:55 |
Qiming | xuhaiwei, in a HTTP response, you will have a header and a body | 07:55 |
xuhaiwei | yes | 07:55 |
Qiming | the header part is for the software to process, while the body part is for the 'user' | 07:55 |
Qiming | if there is a 'header' in the response, the client software can decide whether it will loop-wait until the resource status changes | 07:56 |
*** pratikmallya has quit IRC | 07:56 | |
Qiming | when you are creating a cluster, for example, you return a 202, the body part doesn't matter, because it contains no useful information | 07:56 |
Qiming | but the header should contain a pointer to a location where the client software can check if the resource creation has completed | 07:57 |
Qiming | in the case of cluster-create | 07:57 |
Qiming | the header should contain an entry (as suggested by api-wg): | 07:58 |
Qiming | Location: http://server:port/<tenant_id>/v1/clusters/<cluster_id> | 07:58 |
xuhaiwei | so in that case, all 202 action should include the location header indeed | 07:58 |
Qiming | that is the reason why the guideline says "MUST" | 07:59 |
*** lvdongbing has joined #senlin | 07:59 | |
xuhaiwei | so shall we add location header to all 202 actions or talk with the api-wg guys? | 08:00 |
Qiming | instead the above location can be about the action, instead of the cluster: | 08:00 |
Qiming | Location: http://server:port/<tenant_id>/v1/actions/<action_id> | 08:00 |
Qiming | xuhaiwei, we balance, we may our own judgements here | 08:01 |
xuhaiwei | We treat 'action' as a resource? | 08:01 |
Qiming | actions are resources | 08:01 |
Qiming | we can list and show actions | 08:01 |
Qiming | we RESPECT the guidelines from the api-wg, but they are not GOD | 08:02 |
Qiming | for example, they don't have a lot influences on any projects at all | 08:03 |
Qiming | we are just trying to be good citizens, we do what we believe the right things | 08:03 |
xuhaiwei | ok | 08:03 |
Qiming | if the guidelines doesn't make much senses to use, it is okay to deviate | 08:04 |
Qiming | I believe a lot projects today return 200 for a update command | 08:04 |
Qiming | or they use PUT instead of PATCH even when they do partial update | 08:05 |
Qiming | the world is never perfect | 08:05 |
Qiming | again, we want to be the best citizen, to our best | 08:05 |
xuhaiwei | ok | 08:05 |
xuhaiwei | So according to your comment, first HTTPAccept exception should be removed, right? | 08:06 |
Qiming | yes | 08:06 |
xuhaiwei | and the response body, we should return it, maybe the TODO is not in right place? | 08:07 |
Qiming | using exceptions to return 202 is a hack | 08:07 |
Qiming | we won't get a chance to insert 'Location' header even if we want to | 08:07 |
xuhaiwei | I don't understand | 08:08 |
Qiming | if we are using HTTPAccept exception, we won't get a chance to insert 'Location' header even if we want to | 08:08 |
Qiming | this part okay? | 08:09 |
xuhaiwei | got it | 08:09 |
Qiming | then | 08:09 |
xuhaiwei | miss the context | 08:09 |
xuhaiwei | just now | 08:09 |
xuhaiwei | missed | 08:09 |
Qiming | I don't think we will have a perfect solution for everybody | 08:09 |
Qiming | so my suggestion: | 08:10 |
xuhaiwei | yes? | 08:10 |
Qiming | since api-wg wants us to return the resource in body, we can do that | 08:10 |
Qiming | check the engine-side implementation of cluster-update | 08:11 |
Qiming | the api-wg wants a response to PATCH request to contain a body, I mean, then we do it, it is not a big deal, just some overhead | 08:12 |
Qiming | the 2nd point | 08:12 |
Qiming | since we are returning a 202 for the PATCH verb, there is no guide on this | 08:13 |
Qiming | we follow the 202 rule, add a 'Location' header to the response | 08:13 |
xuhaiwei | ok | 08:13 |
Qiming | I don't think there is a requirement that a 202 response cannot contain a body, right? | 08:14 |
xuhaiwei | yes, currently like creation, we do contain a body | 08:15 |
openstackgerrit | Yanyan Hu proposed openstack/senlin: Merge cluster/node list test to other test cases https://review.openstack.org/252724 | 08:15 |
openstackgerrit | Yanyan Hu proposed openstack/senlin: Revise functional test to adapt to latest API change https://review.openstack.org/252813 | 08:15 |
Qiming | we are not doing that for node-update | 08:15 |
Qiming | engine side implementation of node-update is only returning {'action': action_id} | 08:16 |
Qiming | as a first step | 08:16 |
Qiming | we have the engine side return both a representation of the resource in question, AND an action id | 08:16 |
Qiming | the next step, we will figure out how to build a location pointer at the API layer, based on the action id returned | 08:17 |
Qiming | sounds a plan? | 08:17 |
xuhaiwei | about the location pointer, it should point to the NODE, or ACTION? | 08:17 |
xuhaiwei | for node-update, seems should be node | 08:18 |
Qiming | yep | 08:18 |
Qiming | I think so | 08:18 |
Qiming | but for cluster-resize ? | 08:18 |
xuhaiwei | difficult to decide | 08:19 |
Qiming | yep | 08:19 |
Qiming | https://review.openstack.org/#/c/234994/2/guidelines/actions.rst | 08:19 |
Qiming | still under discussion | 08:19 |
*** zhangguoqing has quit IRC | 08:20 | |
Qiming | we can postpone that decision | 08:20 |
*** zhangguoqing has joined #senlin | 08:20 | |
Qiming | it says this: If the action is started successfully, the server must respond with a 20197 | 08:21 |
Qiming | status code and include a ``Location`` header with the unique URL of the action98 | 08:21 |
Qiming | resource that the client can query for status on the action. | 08:21 |
Qiming | anyway, let's do it step by step | 08:21 |
xuhaiwei | so that should be 'action' | 08:21 |
xuhaiwei | yes | 08:21 |
Qiming | 1st round cleanup of the 'raise HTTPAccept or HTTPNoContent' | 08:22 |
xuhaiwei | ok | 08:22 |
Qiming | 2nd round make sure PATCH contains body as suggested | 08:22 |
Qiming | we'd better return the action id from engine to api if we are gonna return a 202 from API | 08:22 |
Qiming | 3rd round we check how to insert 'Location' header | 08:23 |
xuhaiwei | deal | 08:23 |
Qiming | and if we do this, should it point to the resource or the action | 08:23 |
Qiming | I'm seeing some other problems in the 3rd round | 08:23 |
xuhaiwei | what | 08:24 |
Qiming | if your senlin-api is running behind a proxy server, you will have to be aware of that | 08:24 |
Qiming | you are not supposed to expose your intranet IP directly to the end user | 08:24 |
xuhaiwei | should be an inner IP? | 08:25 |
Qiming | the client sofware is requesting a senlin service on IP1 (a proxy server), which redirects the request to IP2 (internal IP) | 08:25 |
Qiming | your senlin-api is on IP2, and you will return a location for the user to check the resource status | 08:25 |
Qiming | got what I mean? | 08:26 |
Qiming | you cannot say 'Location: http://IP2/.....' in your response | 08:26 |
Qiming | your client software won't be able to reach you | 08:26 |
xuhaiwei | user can't access to it | 08:26 |
Qiming | you cannot say 'Location: http://IP1/.....', because senlin-api never knows whether it is behind a proxy | 08:27 |
Qiming | that is getting dirty | 08:27 |
Qiming | I believe this is why people suggest putting location in header instead of body | 08:28 |
Qiming | a smart proxy server may be able to handle that | 08:28 |
Qiming | anyway, off topic now | 08:28 |
Qiming | Yanyanhu, I hate your patch about changing data types for action | 08:29 |
xuhaiwei | we used to work behind our company's proxy, but now the openstack development is not behind proxy | 08:29 |
Qiming | xuhaiwei, proxy is gonna create a lot problems, :) | 08:29 |
Qiming | I don't think changing from float to REAL is giving you the precision you need | 08:30 |
xuhaiwei | yes, especially when I was using TripleO to construt environment | 08:30 |
Qiming | something else is wrong along the way, Yanyanhu | 08:30 |
Qiming | Yanyanhu, maybe you can check if the value really only contains the integer part | 08:31 |
Qiming | if it is, problem won't get solved by changing it to REAL | 08:31 |
*** yuanying has quit IRC | 08:32 | |
*** yuanying has joined #senlin | 08:32 | |
Yanyanhu | hmm, yes, I think it's also include decimal | 08:32 |
Yanyanhu | hmm, I think you're right, maybe ignore decimal is incorrect | 08:33 |
Yanyanhu | although in most cases, the timeout could not be less than 1 second | 08:33 |
Yanyanhu | will manually set the precision of float type | 08:33 |
Qiming | a float suffices regarding time precision | 08:34 |
Qiming | something else is wrong | 08:34 |
Yanyanhu | looks like with default precision, value will be truncated | 08:34 |
Qiming | you may want to check if the values stored only contains the integer part | 08:34 |
Yanyanhu | if I manually set the precision, it won't happen | 08:35 |
Yanyanhu | hi, Qiming, it contains both integer and decimal | 08:35 |
Qiming | okay | 08:35 |
Qiming | what to you mean by manually set the precision? | 08:35 |
Yanyanhu | I mean something like start_time = sqlalchemy.Column(sqlalchemy.Float('24,8')) | 08:36 |
Qiming | okay, that is dirty | 08:36 |
Yanyanhu | yes | 08:36 |
Qiming | 'sqlalchemy.Float' sounds better than 'REAL' | 08:37 |
Yanyanhu | I tried to find a 'double' type, but it is not supported | 08:37 |
Qiming | I'm supposing 'double' is a C data type | 08:37 |
Qiming | not SQL | 08:37 |
Yanyanhu | yea | 08:38 |
Qiming | when choosing data type to use, please bear in mind we may need to support DB backends other than mysql | 08:38 |
openstackgerrit | xu-haiwei proposed openstack/senlin: Remove HTTPAccept exception from node update API https://review.openstack.org/252787 | 08:38 |
Qiming | so ... pls avoid using MySQL dialect when possible | 08:38 |
Yanyanhu | yes | 08:38 |
openstackgerrit | xu-haiwei proposed openstack/senlin: Remove HTTPAccept exception raised when update cluster https://review.openstack.org/252786 | 08:41 |
openstackgerrit | xu-haiwei proposed openstack/senlin: Remove HTTPNoContent from cluster delete API https://review.openstack.org/252825 | 08:43 |
Qiming | Gentleman, senlin developer docs is online now! | 08:47 |
Qiming | http://docs.openstack.org/developer/openstack-projects.html | 08:47 |
Qiming | oh, there are ladies, ;) | 08:48 |
Qiming | Ladies and gentleman, senlin developer docs is onlin now! | 08:48 |
Yanyanhu | haha | 08:49 |
Yanyanhu | \o/ | 08:49 |
*** lvdongbing has quit IRC | 08:51 | |
*** lvdongbing has joined #senlin | 08:52 | |
Qiming | FYI, filed a bug related to timestamps: https://bugs.launchpad.net/senlin/+bug/1522322 | 08:52 |
openstack | Launchpad bug 1522322 in senlin "exact filtering of timestamps are useless, should be removed" [High,Triaged] | 08:52 |
Yanyanhu | ok, anyone can try to fix it :) | 08:53 |
*** pratikmallya has joined #senlin | 08:57 | |
*** pratikmallya has quit IRC | 09:01 | |
openstackgerrit | Yanyan Hu proposed openstack/senlin: Change data precision of action.start_time and action.end_time https://review.openstack.org/252803 | 09:02 |
openstackgerrit | Yanyan Hu proposed openstack/senlin: Revise functional test to adapt to latest API change https://review.openstack.org/252813 | 09:02 |
openstackgerrit | Yanyan Hu proposed openstack/senlin: Merge cluster/node list test to other test cases https://review.openstack.org/252724 | 09:02 |
openstackgerrit | xu-haiwei proposed openstack/senlin: Remove HTTPAccept exception raised when update cluster https://review.openstack.org/252786 | 09:07 |
openstackgerrit | xu-haiwei proposed openstack/senlin: Remove HTTPAccept exception from node update API https://review.openstack.org/252787 | 09:09 |
xuhaiwei | Yanyanhu | 09:12 |
Yanyanhu | hi | 09:12 |
xuhaiwei | I summitted a patch to remove HTTPNoContent, should I also fix the status_code in function test? | 09:12 |
xuhaiwei | are you doing it? | 09:12 |
xuhaiwei | the DELETE status code should be 202 now | 09:13 |
Yanyanhu | ah, for update operation, you don't since related test cases have not been added | 09:13 |
xuhaiwei | should I do it in the same patch? | 09:13 |
Yanyanhu | for delete or create or action, I guess so | 09:13 |
xuhaiwei | ok, i just need to change the status code to 202? | 09:14 |
Yanyanhu | I just proposed a patch to let functional test match with the latest api change that have been merged | 09:14 |
Yanyanhu | https://review.openstack.org/252813 | 09:14 |
Yanyanhu | yes, I think so | 09:14 |
Yanyanhu | you can do more change after this patch is in | 09:14 |
Yanyanhu | you just need to change something like this: http://git.openstack.org/cgit/openstack/senlin/tree/senlin/tests/functional/api.py#n21 | 09:15 |
Yanyanhu | each time you make revision on an API interface | 09:15 |
xuhaiwei | since you have submitted https://review.openstack.org/252813, I dont need to do it in my patch | 09:15 |
Yanyanhu | yes, for those API change that have been merged, you don't need to do it now | 09:16 |
Yanyanhu | but for some new change like this https://review.openstack.org/252825 | 09:16 |
xuhaiwei | we are depending each other | 09:16 |
Yanyanhu | I guess you may need to change this status_code http://git.openstack.org/cgit/openstack/senlin/tree/senlin/tests/functional/api.py#n70 | 09:17 |
Yanyanhu | ah... | 09:17 |
Qiming | ... hoho | 09:17 |
xuhaiwei | you already changed it in your patch | 09:17 |
Qiming | Yanyanhu, I think funtional tests are yours | 09:17 |
Qiming | xuhaiwei can focus on unit tests | 09:17 |
xuhaiwei | one of us need to rebase after one patch is merged | 09:17 |
Qiming | that will make things smooth | 09:18 |
Yanyanhu | ok, I'm ok with it | 09:18 |
xuhaiwei | ok | 09:18 |
Yanyanhu | so each time you change an API, I will propose a change to functional test | 09:18 |
xuhaiwei | ok | 09:18 |
xuhaiwei | otukare | 09:18 |
Yanyanhu | np | 09:18 |
Yanyanhu | doitashimashita | 09:19 |
Yanyanhu | :) | 09:19 |
xuhaiwei | sogoi | 09:20 |
Yanyanhu | doitashimashite | 09:20 |
Yanyanhu | we are making other people crazy | 09:20 |
Yanyanhu | :P | 09:20 |
xuhaiwei | or you can fix the function test after all the api fix done | 09:20 |
Yanyanhu | sure | 09:21 |
Yanyanhu | just want to follow the API change more tightly to avoid increasing the difficulty of debug | 09:22 |
xuhaiwei | ok, do what you like | 09:22 |
Yanyanhu | yea. since we hope to rely on functional test to find possible issues in some important changes. | 09:23 |
Yanyanhu | although there are still some issue in functional test :) | 09:23 |
*** lvdongbing has quit IRC | 09:34 | |
openstackgerrit | xu-haiwei proposed openstack/senlin: Make node delete request return no body https://review.openstack.org/252851 | 09:53 |
*** pratikmallya has joined #senlin | 09:58 | |
*** pratikmallya has quit IRC | 10:03 | |
*** zhenguo has quit IRC | 10:05 | |
*** lixinhui_ has quit IRC | 10:05 | |
Yanyanhu | prepare to leave, see U guys tomorrow | 10:07 |
*** Yanyanhu has quit IRC | 10:13 | |
*** lvdongbing has joined #senlin | 10:29 | |
*** elynn has quit IRC | 10:40 | |
Qiming | big bang: https://review.openstack.org/#/c/252245/ | 10:50 |
Qiming | see you | 10:54 |
*** Qiming has quit IRC | 10:58 | |
*** yuanying_ has joined #senlin | 11:11 | |
*** yuanying has quit IRC | 11:11 | |
*** pratikmallya has joined #senlin | 11:13 | |
*** yuanying_ has quit IRC | 11:17 | |
*** pratikmallya has quit IRC | 11:33 | |
*** Qiming has joined #senlin | 11:43 | |
*** lvdongbing has quit IRC | 12:08 | |
*** lvdongbing has joined #senlin | 12:24 | |
openstackgerrit | Bertrand Lallau proposed openstack/senlin: Remove kombu as a dependency for Senlin https://review.openstack.org/252924 | 12:31 |
*** openstackgerrit has quit IRC | 12:32 | |
*** zhangguoqing has quit IRC | 12:33 | |
*** openstackgerrit has joined #senlin | 12:33 | |
*** zhangguoqing has joined #senlin | 12:33 | |
*** pratikmallya has joined #senlin | 12:34 | |
*** lixinhui_ has joined #senlin | 12:47 | |
*** pratikmallya has quit IRC | 12:54 | |
*** openstackgerrit has quit IRC | 13:17 | |
*** openstackgerrit has joined #senlin | 13:17 | |
*** elynn has joined #senlin | 13:18 | |
*** pratikmallya has joined #senlin | 13:53 | |
openstackgerrit | Merged openstack/senlin: Change data precision of action.start_time and action.end_time https://review.openstack.org/252803 | 14:02 |
openstackgerrit | Merged openstack/senlin: Merge cluster/node list test to other test cases https://review.openstack.org/252724 | 14:02 |
openstackgerrit | Merged openstack/senlin: Remove HTTPAccept exception raised when update cluster https://review.openstack.org/252786 | 14:02 |
openstackgerrit | Merged openstack/senlin: Remove HTTPAccept exception from node update API https://review.openstack.org/252787 | 14:02 |
*** pratikmallya has quit IRC | 14:14 | |
*** lixinhui_ has quit IRC | 14:17 | |
*** lvdongbing has quit IRC | 14:39 | |
*** Qiming has quit IRC | 15:02 | |
*** pratikmallya has joined #senlin | 15:14 | |
*** pratikmallya has quit IRC | 15:19 | |
*** elynn has quit IRC | 15:31 | |
*** elynn has joined #senlin | 15:34 | |
*** pratikmallya has joined #senlin | 15:59 | |
*** pratikma_ has joined #senlin | 16:14 | |
*** pratikmallya has quit IRC | 16:17 | |
*** elynn has quit IRC | 16:21 | |
*** pratikma_ has quit IRC | 18:01 | |
*** pratikmallya has joined #senlin | 19:00 | |
*** openstackgerrit has quit IRC | 20:47 | |
*** openstackgerrit has joined #senlin | 20:47 | |
*** pratikma_ has joined #senlin | 20:49 | |
*** pratikmallya has quit IRC | 20:49 | |
*** pratikmallya has joined #senlin | 21:34 | |
*** pratikma_ has quit IRC | 21:36 | |
*** yuanying has joined #senlin | 23:21 | |
*** Qiming has joined #senlin | 23:21 | |
*** pratikmallya has quit IRC | 23:24 | |
*** lixinhui_ has joined #senlin | 23:36 | |
*** lixinhui_ has quit IRC | 23:44 | |
*** lixinhui_ has joined #senlin | 23:47 | |
*** Qiming has quit IRC | 23:52 | |
*** pratikmallya has joined #senlin | 23:55 |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!