You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2021/07/24 15:01:43 UTC
[GitHub] [apisix] okaybase opened a new pull request #4661: feat: set the basic id attribute when creating a resource
okaybase opened a new pull request #4661:
URL: https://github.com/apache/apisix/pull/4661
### What this PR does / why we need it:
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
set the basic id attribute when creating a resource
### Pre-submission checklist:
* [x] Did you explain what problem does this PR solve? Or what new features have been added?
* [ ] Have you added corresponding test cases?
* [x] Have you modified the corresponding document?
* [x] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix/tree/master#community) first**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] okaybase commented on a change in pull request #4661: feat: set the basic id attribute when creating a resource via POST
Posted by GitBox <gi...@apache.org>.
okaybase commented on a change in pull request #4661:
URL: https://github.com/apache/apisix/pull/4661#discussion_r676314097
##########
File path: t/admin/routes2.t
##########
@@ -197,6 +197,7 @@ GET /t
res.node.key = nil
res.node.value.create_time = nil
res.node.value.update_time = nil
+ res.node.value.id = nil
Review comment:
the check is done. @spacewander
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] okaybase commented on a change in pull request #4661: feat: set the basic id attribute when creating a resource via POST
Posted by GitBox <gi...@apache.org>.
okaybase commented on a change in pull request #4661:
URL: https://github.com/apache/apisix/pull/4661#discussion_r676247632
##########
File path: t/admin/routes2.t
##########
@@ -197,6 +197,7 @@ GET /t
res.node.key = nil
res.node.value.create_time = nil
res.node.value.update_time = nil
+ res.node.value.id = nil
Review comment:
Thanks for your review firstly. the id (like the key) is generated by the server, and different for every run.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #4661: feat: set the basic id attribute when creating a resource via POST
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4661:
URL: https://github.com/apache/apisix/pull/4661#discussion_r676237518
##########
File path: t/admin/routes2.t
##########
@@ -197,6 +197,7 @@ GET /t
res.node.key = nil
res.node.value.create_time = nil
res.node.value.update_time = nil
+ res.node.value.id = nil
Review comment:
Can we check if id is not nil?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #4661: feat: set the basic id attribute when creating a resource via POST
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4661:
URL: https://github.com/apache/apisix/pull/4661#discussion_r676250251
##########
File path: t/admin/routes2.t
##########
@@ -197,6 +197,7 @@ GET /t
res.node.key = nil
res.node.value.create_time = nil
res.node.value.update_time = nil
+ res.node.value.id = nil
Review comment:
We can just check if it is not nil
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander merged pull request #4661: feat: set the basic id attribute when creating a resource via POST
Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #4661:
URL: https://github.com/apache/apisix/pull/4661
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-chen commented on a change in pull request #4661: feat: set the basic id attribute when creating a resource via POST
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #4661:
URL: https://github.com/apache/apisix/pull/4661#discussion_r676283682
##########
File path: apisix/core/etcd.lua
##########
@@ -311,6 +311,9 @@ function _M.push(key, value, ttl)
local index = res.body.header.revision
index = string.format("%020d", index)
+ -- set the basic id attribute
+ value.id = index
Review comment:
> thanks for your review firstly.
>
> Consumer regards the username property as the identity, so only the HTTP PUT method is supported for creating a new consumer.
>
>
>
> This change is for creating a resource via POST. @nic-chen
👍 thanks for the explanation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] okaybase commented on a change in pull request #4661: feat: set the basic id attribute when creating a resource via POST
Posted by GitBox <gi...@apache.org>.
okaybase commented on a change in pull request #4661:
URL: https://github.com/apache/apisix/pull/4661#discussion_r676247632
##########
File path: t/admin/routes2.t
##########
@@ -197,6 +197,7 @@ GET /t
res.node.key = nil
res.node.value.create_time = nil
res.node.value.update_time = nil
+ res.node.value.id = nil
Review comment:
Thanks for your review firstly. the id (like the key) is generated by the server, and different for every test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] okaybase commented on a change in pull request #4661: feat: set the basic id attribute when creating a resource via POST
Posted by GitBox <gi...@apache.org>.
okaybase commented on a change in pull request #4661:
URL: https://github.com/apache/apisix/pull/4661#discussion_r676268137
##########
File path: apisix/core/etcd.lua
##########
@@ -311,6 +311,9 @@ function _M.push(key, value, ttl)
local index = res.body.header.revision
index = string.format("%020d", index)
+ -- set the basic id attribute
+ value.id = index
Review comment:
thanks for your review firstly.
Consumer regards the username property as the identity, so only the HTTP PUT method is supported for creating a new consumer.
This change is for creating a resource via POST. @nic-chen
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-chen commented on a change in pull request #4661: feat: set the basic id attribute when creating a resource via POST
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #4661:
URL: https://github.com/apache/apisix/pull/4661#discussion_r676262544
##########
File path: apisix/core/etcd.lua
##########
@@ -311,6 +311,9 @@ function _M.push(key, value, ttl)
local index = res.body.header.revision
index = string.format("%020d", index)
+ -- set the basic id attribute
+ value.id = index
Review comment:
There is a problem, the consumer does not have a field `id`. Adding an id directly here will cause the consumer data to become unusable.
@spacewander @okaybase
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] okaybase commented on a change in pull request #4661: feat: set the basic id attribute when creating a resource via POST
Posted by GitBox <gi...@apache.org>.
okaybase commented on a change in pull request #4661:
URL: https://github.com/apache/apisix/pull/4661#discussion_r676252045
##########
File path: t/admin/routes2.t
##########
@@ -197,6 +197,7 @@ GET /t
res.node.key = nil
res.node.value.create_time = nil
res.node.value.update_time = nil
+ res.node.value.id = nil
Review comment:
okay, I will add the check, thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-chen commented on a change in pull request #4661: feat: set the basic id attribute when creating a resource via POST
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #4661:
URL: https://github.com/apache/apisix/pull/4661#discussion_r676262544
##########
File path: apisix/core/etcd.lua
##########
@@ -311,6 +311,9 @@ function _M.push(key, value, ttl)
local index = res.body.header.revision
index = string.format("%020d", index)
+ -- set the basic id attribute
+ value.id = index
Review comment:
There is a problem, the consumer does not have an id. Adding an id directly here will cause the consumer data to become unusable.
@spacewander @okaybase
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on pull request #4661: feat: set the basic id attribute when creating a resource via POST
Posted by GitBox <gi...@apache.org>.
spacewander commented on pull request #4661:
URL: https://github.com/apache/apisix/pull/4661#issuecomment-886306850
FYI @juzhiyuan @nic-chen
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org