You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "nastra (via GitHub)" <gi...@apache.org> on 2024/02/19 15:59:28 UTC
[PR] Core: Don't fail if catalog doesn't support views [iceberg]
nastra opened a new pull request, #9754:
URL: https://github.com/apache/iceberg/pull/9754
(no comment)
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core: Don't fail if catalog doesn't support views [iceberg]
Posted by "amogh-jahagirdar (via GitHub)" <gi...@apache.org>.
amogh-jahagirdar commented on code in PR #9754:
URL: https://github.com/apache/iceberg/pull/9754#discussion_r1494852170
##########
core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java:
##########
@@ -717,8 +718,15 @@ public Transaction createTransaction() {
@Override
public Transaction replaceTransaction() {
- if (viewExists(context, ident)) {
- throw new AlreadyExistsException("View with same name already exists: %s", ident);
+ try {
+ if (viewExists(context, ident)) {
+ throw new AlreadyExistsException("View with same name already exists: %s", ident);
+ }
+ } catch (RESTException | UnsupportedOperationException e) {
+ // don't fail if the server doesn't support views, which could be due to:
+ // 1. server or backing catalog doesn't support views
+ // 2. newer client talks to an older server that doesn't support views
+ LOG.debug("Could not check whether view exists: {}", ident, e);
Review Comment:
"Could not determine if view {} already exists"?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core: Don't fail if catalog doesn't support views [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #9754:
URL: https://github.com/apache/iceberg/pull/9754#discussion_r1494850470
##########
core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java:
##########
@@ -717,8 +718,15 @@ public Transaction createTransaction() {
@Override
public Transaction replaceTransaction() {
- if (viewExists(context, ident)) {
- throw new AlreadyExistsException("View with same name already exists: %s", ident);
+ try {
+ if (viewExists(context, ident)) {
+ throw new AlreadyExistsException("View with same name already exists: %s", ident);
+ }
+ } catch (RESTException | UnsupportedOperationException e) {
+ // don't fail if the server doesn't support views, which could be due to:
+ // 1. server or backing catalog doesn't support views
+ // 2. newer client talks to an older server that doesn't support views
+ LOG.debug("Could not check whether view {} exists: {}", ident, e.getMessage(), e);
Review Comment:
updated
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core: Don't fail if catalog doesn't support views [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on PR #9754:
URL: https://github.com/apache/iceberg/pull/9754#issuecomment-1953263483
Thanks, @nastra!
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core: Don't fail if catalog doesn't support views [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #9754:
URL: https://github.com/apache/iceberg/pull/9754#discussion_r1494858016
##########
core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java:
##########
@@ -717,8 +718,15 @@ public Transaction createTransaction() {
@Override
public Transaction replaceTransaction() {
- if (viewExists(context, ident)) {
- throw new AlreadyExistsException("View with same name already exists: %s", ident);
+ try {
+ if (viewExists(context, ident)) {
+ throw new AlreadyExistsException("View with same name already exists: %s", ident);
+ }
+ } catch (RESTException | UnsupportedOperationException e) {
+ // don't fail if the server doesn't support views, which could be due to:
+ // 1. server or backing catalog doesn't support views
+ // 2. newer client talks to an older server that doesn't support views
+ LOG.debug("Could not check whether view exists: {}", ident, e);
Review Comment:
I've updated this to `Failed to check whether view {} exists`
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core: Don't fail if catalog doesn't support views [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #9754:
URL: https://github.com/apache/iceberg/pull/9754#discussion_r1494846660
##########
core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java:
##########
@@ -717,8 +718,15 @@ public Transaction createTransaction() {
@Override
public Transaction replaceTransaction() {
- if (viewExists(context, ident)) {
- throw new AlreadyExistsException("View with same name already exists: %s", ident);
+ try {
+ if (viewExists(context, ident)) {
+ throw new AlreadyExistsException("View with same name already exists: %s", ident);
+ }
+ } catch (RESTException | UnsupportedOperationException e) {
+ // don't fail if the server doesn't support views, which could be due to:
+ // 1. server or backing catalog doesn't support views
+ // 2. newer client talks to an older server that doesn't support views
+ LOG.debug("Could not check whether view {} exists: {}", ident, e.getMessage(), e);
Review Comment:
Style: typically we would use "Failed to check whether view {} exists" and leave the message to the exception trace.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core: Don't fail if catalog doesn't support views [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue merged PR #9754:
URL: https://github.com/apache/iceberg/pull/9754
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org