paul-rogers commented on pull request #2353: URL: https://github.com/apache/drill/pull/2353#issuecomment-976015866
@CuteKittyhoho, thanks for this PR. I wonder, can we simplify the API a bit. As you stated your change is to change the original `/profiles.json` to: ```text /profiles.json /profiles/running /profiles/completed ``` One could imagine other filters as well: failed requests, completed requests, running requests by Charles, etc. So, I wonder if it makes sense to add the filter as a query option: `/profiles/json?status=[all|running|completed]&...` Where the `...` is whatever someone else wants to add later. Might make the code a bit simpler also. Then, on the unit tests: testing this one might be hard: there is no good way to ensure that there are, in fact, running queries when you request them. If it were me, I'd add a hack to the mock data source. Add a global lock. By default, it is unlocked so existing code works. A test can set the lock, then launch a query in one thread, while testing your API in another. The mock data source, when it fetches a batch, would block if the lock is set, waiting for the lock to be cleared. Ugly, but it should work. Now, I'm not sure it's fair for us to ask you to add all that for just this case, so I guess I'm OK with doing only manual ad-hoc tests for now. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
