-
Notifications
You must be signed in to change notification settings - Fork 110
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
transition to cli errors #785
Conversation
Merge branch 'main' into cnd-627 # Conflicts: # R/connection-pane.R
@@ -14,11 +14,10 @@ OdbcConnection <- function( | |||
timeout = Inf, | |||
dbms.name = NULL, | |||
attributes = NULL, | |||
.connection_string = NULL | |||
.connection_string = NULL, | |||
call = caller_env(2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using caller_env(2)
as caller_env()
shows an unhelpful ".local()" frame:
test_con("SQLITE", attributes = list(boop = "bop"))
#> Error in `.local()`:
#> ! `attributes` does not support the connection attribute "boop".
#> ℹ Allowed connection attribute is "azure_token".
with:
3. └─odbc::dbConnect(...)
4. └─odbc (local) .local(drv, ...)
5. └─odbc:::OdbcConnection(...) at odbc/R/dbi-driver.R:184:5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, S4 adds the intermediate.local
wrapper when the arguments of the generic and the method differ in some way. I think it happens here because our method adds the dsn
argument before the docs. (No need to change anything, I just thought you might want to know where this was coming from)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good!
@@ -14,11 +14,10 @@ OdbcConnection <- function( | |||
timeout = Inf, | |||
dbms.name = NULL, | |||
attributes = NULL, | |||
.connection_string = NULL | |||
.connection_string = NULL, | |||
call = caller_env(2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, S4 adds the intermediate.local
wrapper when the arguments of the generic and the method differ in some way. I think it happens here because our method adds the dsn
argument before the docs. (No need to change anything, I just thought you might want to know where this was coming from)
call = call | ||
) | ||
} | ||
check_exclusive(table, view, .frame = call) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:chef kiss:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SUCH A SATISFYING DIFF
Is there a particular reason why some calls to I must say that I personally am not terribly happy with the switch to using |
Closes #627.