we can create a Github issue and continue this job. ^_^
On Thu, Jul 16, 2020 at 8:50 PM junxu chen <[email protected]> wrote: > Hello, Yuansheng, > Your understanding is right, and "array object should update fully" looks > good to me. > > On Thu, Jul 16, 2020 at 10:01 AM YuanSheng Wang <[email protected]> > wrote: > > > @junxu I think this way is enough. > > > > Here is more detail, please confirm I am right: > > > > > update `methods` fully: > > > curl -XPATCH http://172.17.0.1:9080/apisix/admin/upstreams/1/methods > -d > > ‘["GET"]’ > > > > fully update data by sub-path, eg: `/methods` > > > > Here is case A: curl -XPATCH > > http://172.17.0.1:9080/apisix/admin/upstreams/1/nodes -d ‘{" > 127.0.0.1:8080 > > ": > > 10}’ > > > > Is this a full update or a partial update? I prefer it is full updating. > > > > > Self register and not affect other nodes should be: > > > curl -XPATCH http://172.17.0.1:9080/apisix/admin/upstreams/1 -d > ‘{nodes: > > {"172.17.0.6:8080": 10}}’ > > > Self unregister and not affect other nodes should be: > > > curl -XPATCH http://172.17.0.1:9080/apisix/admin/upstreams/1 -d > ‘{nodes: > > {"172.17.0.6:8080": null}}’ > > > > agree with this, for the non-array object, it should be a partial > updating. > > > > But I want to confirm some detail, please take a look at the case `B`: > > case `B`: curl -XPATCH http://172.17.0.1:9080/apisix/admin/upstreams/1-d > > ‘{" > > methods":["GET"]}’ > > > > For the array object, we should update the `methods` in full. > > If the old `methods` value is '["GET", "POST"]', the new value should be > ` > > ["GET"]`. > > > > > > On Tue, Jul 14, 2020 at 1:38 PM junxu chen <[email protected]> wrote: > > > > > Sorry for the mistake. > > > > > > Self register and not affect other nodes should be: > > > > > > curl -XPATCH http://172.17.0.1:9080/apisix/admin/upstreams/1 > > > <http://172.17.0.1:9080/apisix/admin/upstreams/1/nodes> -d ‘{nodes: {" > > > 172.17.0.6:8080": 10}}’ > > > > > > > > > Self unregister and not affect other nodes should be: > > > > > > curl -XPATCH http://172.17.0.1:9080/apisix/admin/upstreams/1 > > > <http://172.17.0.1:9080/apisix/admin/upstreams/1/nodes> -d ‘{nodes: {" > > > 172.17.0.6:8080": null}}’ > > > > > > > > > > > > On Tue, Jul 14, 2020 at 1:19 PM junxu chen <[email protected]> > wrote: > > > > > > > hi, Ming > > > > > > > > If we support both styles, it should be: > > > > > > > > curl -XPATCH http://172.17.0.1:9080/apisix/admin/upstreams/1/methods > > -d > > > > ‘["GET"]’ > > > > > > > > > > > > Self register and not affect other nodes: > > > > > > > > curl -XPATCH http://172.17.0.1:9080/apisix/admin/upstreams/1/nodes > -d > > > ‘{" > > > > 172.17.0.6:8080": 10}’ > > > > > > > > > > > > Self unregister and not affect other nodes: > > > > > > > > curl -XPATCH http://172.17.0.1:9080/apisix/admin/upstreams/1/nodes > -d > > > ‘{" > > > > 172.17.0.6:8080": null}’ > > > > > > > > > > > > > > > > Multiple implementations do confuse users, but it should be better > than > > > > not meeting the needs. . > > > > > > > > > > > > On Tue, Jul 14, 2020 at 9:51 AM Ming Wen <[email protected]> wrote: > > > > > > > >> hi,junxu, > > > >> please show the example how to resolve: > > > >> "methods": ["GET", null, null, null, null, null, null, null, null] > > > >> > > > >> IMO, multiple implementations will confuse users. > > > >> > > > >> Thanks, > > > >> Ming Wen, Apache APISIX(incubating) & Apache SkyWalking > > > >> Twitter: _WenMing > > > >> > > > >> > > > >> junxu chen <[email protected]> 于2020年7月14日周二 上午9:28写道： > > > >> > > > >> > I think We could support both styles. > > > >> > Want to update a certain attribute in full, use the old style. > > > >> > Want to partially update, use the current style. > > > >> > > > > >> > On Tue, Jul 14, 2020 at 9:15 AM Ming Wen <[email protected]> > > wrote: > > > >> > > > > >> > > > For example, if user want to update the `method` of > > > >> > > `/apisix/admin/routes/1`, > > > >> > > user need to PATCH with data: `"methods": ["GET", null, null, > > null, > > > >> null, > > > >> > > null, null, null, null]`. For me, I don't know why I need a lot > of > > > >> `null` > > > >> > > after "GET". > > > >> > > > > > >> > > I suggest we focus on solving these kinds of problems first. > > > >> > > > > > >> > > Thanks, > > > >> > > Ming Wen, Apache APISIX(incubating) & Apache SkyWalking > > > >> > > Twitter: _WenMing > > > >> > > > > > >> > > > > > >> > > YuanSheng Wang <[email protected]> 于2020年7月14日周二 上午8:52写道： > > > >> > > > > > >> > > > old style: > > > >> > > > curl -XPATCH > > http://127.0.0.1:9080/apisix/admin/upstreams/1/nodes > > > >> -d > > > >> > ‘{" > > > >> > > > 127.0.0.1:8083":3}’ > > > >> > > > > > > >> > > > current style: > > > >> > > > curl -XPATCH http://127.0.0.1:9080/apisix/admin/upstreams/1 > -d > > > >> > ‘{nodes: > > > >> > > {" > > > >> > > > 127.0.0.1:8083":3}}’ > > > >> > > > > > > >> > > > They are the same and all idempotent. > > > >> > > > > > > >> > > > > > > >> > > > On Tue, Jul 14, 2020 at 7:27 AM Ming Wen <[email protected]> > > > >> wrote: > > > >> > > > > > > >> > > > > hi, jinwei, > > > >> > > > > we need to roll back the current PATCH implementation if you > > > want > > > >> > this > > > >> > > > > style of admin api. > > > >> > > > > > > > >> > > > > > > > >> > > > > jinwei <[email protected]> 于 2020年7月14日周二 上午12:25写道： > > > >> > > > > > > > >> > > > > > I used to use this API a lot > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > curl -XPATCH > > > >> http://127.0.0.1:9080/apisix/admin/upstreams/1/nodes > > > >> > -d > > > >> > > > ‘{" > > > >> > > > > > 127.0.0.1:8083":3}’ > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > I like this API very much, because it is idempotent. We > can > > > >> clearly > > > >> > > > know > > > >> > > > > > that the result of nodes is the element I specify and will > > not > > > >> be > > > >> > > > > affected > > > >> > > > > > by history; > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > This API is also useful for service registration and > > > discovery ！ > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > Hope to keep this API > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > Thank you very much > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > At 2020-07-13 22:25:43, "YuanSheng Wang" < > > [email protected] > > > > > > > >> > > wrote: > > > >> > > > > > >{ > > > >> > > > > > > desc: xxxx, > > > >> > > > > > > id: xxxx, > > > >> > > > > > > nodes: ["xx", "yy", "zz"] > > > >> > > > > > >} > > > >> > > > > > > > > > >> > > > > > >I have one question, if we want to update the `desc` and > > > >> `nodes`, > > > >> > > how > > > >> > > > to > > > >> > > > > > do > > > >> > > > > > >with this case? > > > >> > > > > > >The old API style does not support this way. > > > >> > > > > > > > > > >> > > > > > >Should we support this case? Otherwise, we will never > > support > > > >> > > updating > > > >> > > > > > part > > > >> > > > > > >of the data like this. > > > >> > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > >> > > > > > >On Mon, Jul 13, 2020 at 10:52 AM Ming Wen < > > > [email protected]> > > > >> > > wrote: > > > >> > > > > > > > > > >> > > > > > >> Agreed, it's acceptable. We should keep user-friendly. > > > >> > > > > > >> > > > >> > > > > > >> Thanks, > > > >> > > > > > >> Ming Wen, Apache APISIX(incubating) & Apache SkyWalking > > > >> > > > > > >> Twitter: _WenMing > > > >> > > > > > >> > > > >> > > > > > >> > > > >> > > > > > >> Zhiyuan Ju <[email protected]> 于2020年7月13日周一 > > 上午6:42写道： > > > >> > > > > > >> > > > >> > > > > > >> > I think when facing the issue you mentioned, we just > > > >> > > > > > >> > > > > >> > > > > > >> > PATCH {methods: [GET, POST]} > > > >> > > > > > >> > > > > >> > > > > > >> > , and API should just do a “PUT Like” action for the > > > >> “methods” > > > >> > > > > filed. > > > >> > > > > > >> > > > > >> > > > > > >> > Data with some fixed length “null” is confusing > > actually. > > > >> > > > > > >> > > > > >> > > > > > >> > Ming Wen <[email protected]>于2020年7月12日 > 周日下午10:45写道： > > > >> > > > > > >> > > > > >> > > > > > >> > > Whether to roll back has nothing to do with new or > > old > > > >> > > commit. > > > >> > > > > > >> > > > > > >> > > > > > >> > > The current implementation is not in compliance > with > > > the > > > >> > > > > > specifications > > > >> > > > > > >> > and > > > >> > > > > > >> > > user perception, there is no need to keep. > > > >> > > > > > >> > > > > > >> > > > > > >> > > APISIX is API gateway, the admin api must follow > good > > > >> design > > > >> > > > > > >> > > specifications. > > > >> > > > > > >> > > > > > >> > > > > > >> > > YuanSheng Wang <[email protected]> 于 > 2020年7月12日周日 > > > >> > > 下午10:13写道： > > > >> > > > > > >> > > > > > >> > > > > > >> > > > It is not a good idea to `roll back` the PATCH > > > >> > > implementation > > > >> > > > > for > > > >> > > > > > >> admin > > > >> > > > > > >> > > > API. > > > >> > > > > > >> > > > > > > >> > > > > > >> > > > 1. it is an old commit. > > > >> > > > > > >> > > > 2. we can support the sub `PATH` if we need to > > > support > > > >> it. > > > >> > > > > > >> > > > > > > >> > > > > > >> > > > > > > >> > > > > > >> > > > On Sun, Jul 12, 2020 at 10:07 PM Ming Wen < > > > >> > > [email protected] > > > >> > > > > > > > >> > > > > > >> wrote: > > > >> > > > > > >> > > > > > > >> > > > > > >> > > > > I think the design of admin api should refer to > > > >> google > > > >> > API > > > >> > > > > > design > > > >> > > > > > >> > > doc[1], > > > >> > > > > > >> > > > > and this makes it easy to reach consensus with > > > users. > > > >> > > > > > >> > > > > > > > >> > > > > > >> > > > > [1] > > > >> > https://cloud.google.com/apis/design/standard_methods > > > >> > > > > > >> > > > > > > > >> > > > > > >> > > > > Thanks, > > > >> > > > > > >> > > > > Ming Wen, Apache APISIX(incubating) & Apache > > > >> SkyWalking > > > >> > > > > > >> > > > > Twitter: _WenMing > > > >> > > > > > >> > > > > > > > >> > > > > > >> > > > > > > > >> > > > > > >> > > > > Ming Wen <[email protected]> 于2020年7月12日周日 > > > >> 下午9:56写道： > > > >> > > > > > >> > > > > > > > >> > > > > > >> > > > > > hello, all, > > > >> > > > > > >> > > > > > A user has reported a issue[1] about PATCH > > method > > > >> of > > > >> > > admin > > > >> > > > > > API. > > > >> > > > > > >> > > > > > I looked at the PR[2] that was causing user > > > >> confusion, > > > >> > > > and I > > > >> > > > > > >> think > > > >> > > > > > >> > > the > > > >> > > > > > >> > > > > > user is using it in the right way and our > > > >> > implementation > > > >> > > > is > > > >> > > > > > >> > > > > inappropriate. > > > >> > > > > > >> > > > > > > > > >> > > > > > >> > > > > > For example, if user want to update the > > `method` > > > of > > > >> > > > > > >> > > > > `/apisix/admin/routes/1`, > > > >> > > > > > >> > > > > > user need to PATCH with data: `"methods": > > ["GET", > > > >> > null, > > > >> > > > > null, > > > >> > > > > > >> null, > > > >> > > > > > >> > > > null, > > > >> > > > > > >> > > > > > null, null, null, null]`. For me, I don't > know > > > why > > > >> I > > > >> > > need > > > >> > > > a > > > >> > > > > > lot > > > >> > > > > > >> of > > > >> > > > > > >> > > > `null` > > > >> > > > > > >> > > > > > after "GET". > > > >> > > > > > >> > > > > > > > > >> > > > > > >> > > > > > From the user's perspective, the current > > > >> > implementation > > > >> > > is > > > >> > > > > not > > > >> > > > > > >> > > > > > appropriate. So I suggest roll back the > > current > > > >> PATCH > > > >> > > > > > >> > > > implementation[2] > > > >> > > > > > >> > > > > > for admin api. > > > >> > > > > > >> > > > > > > > > >> > > > > > >> > > > > > what do you think? > > > >> > > > > > >> > > > > > > > > >> > > > > > >> > > > > > [1] > > > >> > > > https://github.com/apache/incubator-apisix/issues/1823 > > > >> > > > > > >> > > > > > [2] > > > >> > > https://github.com/apache/incubator-apisix/pull/1609 > > > >> > > > > > >> > > > > > [3] > > > >> > > > > > >> > > > > > > > > >> > > > > > >> > > > > > > > >> > > > > > >> > > > > > > >> > > > > > >> > > > > > >> > > > > > >> > > > > >> > > > > > >> > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > >> > > > > > > https://github.com/apache/incubator-apisix/pull/1609/files#diff-00625723b6e737f3cdb18af67165b70fR996 > > > >> > > > > > >> > > > > > > > > >> > > > > > >> > > > > > Thanks, > > > >> > > > > > >> > > > > > Ming Wen, Apache APISIX(incubating) & Apache > > > >> > SkyWalking > > > >> > > > > > >> > > > > > Twitter: _WenMing > > > >> > > > > > >> > > > > > > > > >> > > > > > >> > > > > > > > >> > > > > > >> > > > > > > >> > > > > > >> > > > > > > >> > > > > > >> > > > -- > > > >> > > > > > >> > > > > > > >> > > > > > >> > > > *MembPhis* > > > >> > > > > > >> > > > My GitHub: https://github.com/membphis > > > >> > > > > > >> > > > Apache APISIX: > > > >> https://github.com/apache/incubator-apisix > > > >> > > > > > >> > > > > > > >> > > > > > >> > > > > > >> > > > > > >> > -- > > > >> > > > > > >> > 来自 琚致远 > > > >> > > > > > >> > > > > >> > > > > > >> > > > >> > > > > > > > > > >> > > > > > > > > > >> > > > > > >-- > > > >> > > > > > > > > > >> > > > > > >*MembPhis* > > > >> > > > > > >My GitHub: https://github.com/membphis > > > >> > > > > > >Apache APISIX: > https://github.com/apache/incubator-apisix > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > > >> > > > -- > > > >> > > > > > > >> > > > *MembPhis* > > > >> > > > My GitHub: https://github.com/membphis > > > >> > > > Apache APISIX: https://github.com/apache/incubator-apisix > > > >> > > > > > > >> > > > > > >> > > > > >> > > > > > > > > > > > > > -- > > > > *MembPhis* > > My GitHub: https://github.com/membphis > > Apache APISIX: https://github.com/apache/incubator-apisix > > > -- *MembPhis* My GitHub: https://github.com/membphis Apache APISIX: https://github.com/apache/incubator-apisix
