Hi Ismael, Thanks for the review. See my responses below.
One potential issue is that the number of topics in the response won't > match the number of topics in the request. Worth checking what others think > of this one. Based on the choice of how we handled duplicate topics in the create topics protocol, I took the same approach here. At one point create topics would disconnect because I could't return an error per topic request. In the end the preference was to return and error code even though the cardinality would be different. 4. When requesting to delete a topic that is already marked for > > > deletion, the request will wait for the wait for the timeout and > > return as > > > usual > > Do you mean that it will wait up to the timeout until the delete is > "complete" as per the definition in `6`? Or will it wait unconditionally > until the timeout expires? It would be good to make that clear. > That's exactly what I meant. I updated the wiki. This could leak topic name information (as per KAFKA-3396, which was filed > by you). We would probably want to return `InvalidTopic` for the case where > the user doesn't have a valid `DESCRIBE TOPIC` ACL, right? Good point. I will update the wiki and patch. Unauthorized requests will receive a TopicAuthorizationException if they are authorized to the "Describe" Operation on the "Topic" resource. Otherwise they will receive an InvalidTopicException as if the topic does not exist. I was wondering (and this applies to the create topic as well), is there > any value in a flag that says whether the timeout expired or not? In both the create and delete response we return a TimeoutException error code for the topics that did not "complete" in time. This allows the client to know which topics actions completed and which timed out. I updated the wiki to explicitly call this out in the response section. Thanks, Grant On Tue, Jun 21, 2016 at 5:44 AM, Ismael Juma <ism...@juma.me.uk> wrote: > Thanks Grant. A few comments inline. > > On Mon, Jun 20, 2016 at 9:09 PM, Grant Henke <ghe...@cloudera.com> wrote: > > > > 2. If there are multiple instructions for the same topic in one > > > request the extra request will be ignored > > > - This is because the list of topics is modeled server side as a > > set > > > - Multiple deletes results in the same end goal, so handling this > > > error for the user should be okay > > > > One potential issue is that the number of topics in the response won't > match the number of topics in the request. Worth checking what others think > of this one. > > 4. When requesting to delete a topic that is already marked for > > > deletion, the request will wait for the wait for the timeout and > > return as > > > usual > > > Do you mean that it will wait up to the timeout until the delete is > "complete" as per the definition in `6`? Or will it wait unconditionally > until the timeout expires? It would be good to make that clear. > > > > > 5. The principal must be authorized to the "Delete" Operation on the > > > > "Topic" resource to delete the topic. > > > - Unauthorized requests will receive a > TopicAuthorizationException > > > > This could leak topic name information (as per KAFKA-3396, which was filed > by you). We would probably want to return `InvalidTopic` for the case where > the user doesn't have a valid `DESCRIBE TOPIC` ACL, right? > > > > > - Why have a timeout at all? Deletes could take a while? > > > > I was wondering (and this applies to the create topic as well), is there > any value in a flag that says whether the timeout expired or not? > > Thanks, > Ismael > -- Grant Henke Software Engineer | Cloudera gr...@cloudera.com | twitter.com/gchenke | linkedin.com/in/granthenke
