You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by GitBox <gi...@apache.org> on 2020/10/27 06:18:57 UTC
[GitHub] [calcite-avatica] john-bodley opened a new pull request #129: [doc] Fix RpcMetadata, DatabasePropertyRequest, et al.
john-bodley opened a new pull request #129:
URL: https://github.com/apache/calcite-avatica/pull/129
Whilst using the Avatica JSON API I noticed a couple of issues with the documentation:
1. The `DatabasePropertyRequest` was missing the `connection_id` in the example signature.
2. `RpcMetadata` is actually a response as opposed to a miscellaneous type per [here](https://github.com/apache/calcite-avatica/blob/4b7eee5bf430b916c7c07897b6f60d2b6b6dabb7/core/src/main/protobuf/responses.proto#L114-L116) and thus requires a `response` field. Note I'm not certain if this was intentional, i.e., it being a response, however it it is it seems that itt should be renamed to `RpcMetadataResponse` for consistency.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite-avatica] joshelser commented on pull request #129: [CALCITE-4367] Fix RpcMetadata, DatabasePropertyRequest, et al. documentation (john-bodley)
Posted by GitBox <gi...@apache.org>.
joshelser commented on pull request #129:
URL: https://github.com/apache/calcite-avatica/pull/129#issuecomment-779562301
@F21 @danny0405 what's going on here? We have two pull-requests for this one Jira issue? see #131
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite-avatica] danny0405 commented on pull request #129: [doc] Fix RpcMetadata, DatabasePropertyRequest, et al.
Posted by GitBox <gi...@apache.org>.
danny0405 commented on pull request #129:
URL: https://github.com/apache/calcite-avatica/pull/129#issuecomment-719149819
> @danny0405 I also noticed a few values which were not marked as optional but the Avatica JDBC driver presented as `null`.
Thanks, please fix it also and update the commit message as "[CALCITE-{JIRA number}] main message (your github account name)".
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite-avatica] F21 commented on pull request #129: [CALCITE-4367] Fix RpcMetadata, DatabasePropertyRequest, et al. documentation (john-bodley)
Posted by GitBox <gi...@apache.org>.
F21 commented on pull request #129:
URL: https://github.com/apache/calcite-avatica/pull/129#issuecomment-779564607
@joshelser After looking at your second comment in the jira issue, perhaps the changes in this PR are not needed? Can you please confirm?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite-avatica] danny0405 commented on pull request #129: [CALCITE-4367] Fix RpcMetadata, DatabasePropertyRequest, et al. documentation
Posted by GitBox <gi...@apache.org>.
danny0405 commented on pull request #129:
URL: https://github.com/apache/calcite-avatica/pull/129#issuecomment-719158804
> @danny0405 I've made the updates you requested.
Hmm, you missed the GitHub account, and please also squish the commits into one.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite-avatica] F21 commented on pull request #129: [CALCITE-4367] Fix RpcMetadata, DatabasePropertyRequest, et al. documentation (john-bodley)
Posted by GitBox <gi...@apache.org>.
F21 commented on pull request #129:
URL: https://github.com/apache/calcite-avatica/pull/129#issuecomment-779565765
Thanks! I'll go ahead and close this.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite-avatica] joshelser commented on pull request #129: [CALCITE-4367] Fix RpcMetadata, DatabasePropertyRequest, et al. documentation (john-bodley)
Posted by GitBox <gi...@apache.org>.
joshelser commented on pull request #129:
URL: https://github.com/apache/calcite-avatica/pull/129#issuecomment-779565476
> Can you please confirm?
Glancing through a couple of the changes, I think I re-implemented all of them. The author didn't link this PR to the Jira issue, so I had no idea he actually made these changes :(. I think this can just be closed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite-avatica] john-bodley commented on a change in pull request #129: [doc] Fix RpcMetadata, DatabasePropertyRequest, et al.
Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #129:
URL: https://github.com/apache/calcite-avatica/pull/129#discussion_r514807938
##########
File path: site/_docs/json_reference.md
##########
@@ -920,6 +934,7 @@ This object represents the properties for a given JDBC Connection.
{% highlight json %}
{
"connProps": "connPropsImpl",
+ "dirty": true,
Review comment:
This is [`is_dirty`](https://github.com/apache/calcite-avatica/blob/4b7eee5bf430b916c7c07897b6f60d2b6b6dabb7/core/src/main/protobuf/common.proto#L24) for protobuf but seems to be merely `dirty` for JSON.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite-avatica] danny0405 commented on pull request #129: [doc] Fix RpcMetadata, DatabasePropertyRequest, et al.
Posted by GitBox <gi...@apache.org>.
danny0405 commented on pull request #129:
URL: https://github.com/apache/calcite-avatica/pull/129#issuecomment-719124241
Can you log an issue there so that it can be tracked by the JIRA.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite-avatica] joshelser commented on pull request #129: [CALCITE-4367] Fix RpcMetadata, DatabasePropertyRequest, et al. documentation (john-bodley)
Posted by GitBox <gi...@apache.org>.
joshelser commented on pull request #129:
URL: https://github.com/apache/calcite-avatica/pull/129#issuecomment-779563989
> This one has a few more changes in addition to the changes..
Can we get a new Jira issue open before any changes get merged, please? Will be good to keep the release note accurate 🙏🏼
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite-avatica] john-bodley commented on pull request #129: [CALCITE-4367] Fix RpcMetadata, DatabasePropertyRequest, et al. documentation
Posted by GitBox <gi...@apache.org>.
john-bodley commented on pull request #129:
URL: https://github.com/apache/calcite-avatica/pull/129#issuecomment-719157786
@danny0405 I've made the updates you requested.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite-avatica] john-bodley commented on pull request #129: [doc] Fix RpcMetadata, DatabasePropertyRequest, et al.
Posted by GitBox <gi...@apache.org>.
john-bodley commented on pull request #129:
URL: https://github.com/apache/calcite-avatica/pull/129#issuecomment-719144079
@danny0405 I also noticed a few values which were not marked as optional but the Avatica JDBC driver presented as `null`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite-avatica] F21 commented on pull request #129: [CALCITE-4367] Fix RpcMetadata, DatabasePropertyRequest, et al. documentation (john-bodley)
Posted by GitBox <gi...@apache.org>.
F21 commented on pull request #129:
URL: https://github.com/apache/calcite-avatica/pull/129#issuecomment-779463966
@danny0405 Can you please take a look at this?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite-avatica] F21 commented on pull request #129: [CALCITE-4367] Fix RpcMetadata, DatabasePropertyRequest, et al. documentation (john-bodley)
Posted by GitBox <gi...@apache.org>.
F21 commented on pull request #129:
URL: https://github.com/apache/calcite-avatica/pull/129#issuecomment-779563694
hmm. This one has a few more changes in addition to the changes from #131.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite-avatica] F21 closed pull request #129: [CALCITE-4367] Fix RpcMetadata, DatabasePropertyRequest, et al. documentation (john-bodley)
Posted by GitBox <gi...@apache.org>.
F21 closed pull request #129:
URL: https://github.com/apache/calcite-avatica/pull/129
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org