Skip to content
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

error when calling %sqlcmd snippets if there is no active connection #761

Closed
edublancas opened this issue Jul 27, 2023 · 4 comments · Fixed by #804
Closed

error when calling %sqlcmd snippets if there is no active connection #761

edublancas opened this issue Jul 27, 2023 · 4 comments · Fixed by #804
Assignees

Comments

@edublancas
Copy link

%load_ext sql
%sqlcmd snippets

error:

---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
Cell In[3], line 1
----> 1 get_ipython().run_line_magic('sqlcmd', 'snippets')

File ~/miniconda3/envs/jupysql/lib/python3.10/site-packages/IPython/core/interactiveshell.py:2417, in InteractiveShell.run_line_magic(self, magic_name, line, _stack_depth)
   2415     kwargs['local_ns'] = self.get_local_scope(stack_depth)
   2416 with self.builtin_trap:
-> 2417     result = fn(*args, **kwargs)
   2419 # The code below prevents the output from being displayed
   2420 # when using magics with decodator @output_can_be_silenced
   2421 # when the last Python token in the expression is a ';'.
   2422 if getattr(fn, magic.MAGIC_OUTPUT_CAN_BE_SILENCED, False):

File ~/dev/jupysql/src/sql/magic_cmd.py:45, in SqlCmdMagic._validate_execute_inputs(self, line)
     39 """
     40 Function to validate %sqlcmd inputs.
     41 Raises UsageError in case of an invalid input, executes command otherwise.
     42 """
     44 # We rely on SQLAlchemy when inspecting tables
---> 45 util.support_only_sql_alchemy_connection("%sqlcmd")
     47 AVAILABLE_SQLCMD_COMMANDS = [
     48     "tables",
     49     "columns",
   (...)
     53     "snippets",
     54 ]
     56 VALID_COMMANDS_MSG = (
     57     f"Missing argument for %sqlcmd. "
     58     f"Valid commands are: {', '.join(AVAILABLE_SQLCMD_COMMANDS)}"
     59 )

File ~/dev/jupysql/src/sql/util.py:252, in support_only_sql_alchemy_connection(command)
    248 def support_only_sql_alchemy_connection(command):
    249     """
    250     Throws a sql.exceptions.RuntimeError if connection is not SQLAlchemy
    251     """
--> 252     if ConnectionManager.current.is_dbapi_connection:
    253         raise exceptions.RuntimeError(
    254             f"{command} is only supported with SQLAlchemy "
    255             "connections, not with DBAPI connections"
    256         )

AttributeError: 'NoneType' object has no attribute 'is_dbapi_connection'
@bbeat2782
Copy link

bbeat2782 commented Aug 13, 2023

While I was working on #783, I noticed that this error also occurs for other %sqlcmd arguments.

Acceptance Criteria

  1. Raise an exceptions error that tells there is no active connection when a user runs tables, columns, test, profile, explore from %sqlcmd command without an active connection
    a. If there is connection, it should follow the current behavior (raise an error from util.support_only_sql_alchemy_connection if the given command only works with sqlalchemy connection)
  2. When a user runs snippets, connect from %sqlcmd command without an active connection
    a. snippets: No snippets stored
    b. connect: display the widget
  3. Add test functions to test the error and message

@bbeat2782 bbeat2782 self-assigned this Aug 13, 2023
@edublancas
Copy link
Author

@bbeat2782 thanks for working on this.

this happens because some commands only work with sqlalchemy connections. so we should still raise the error. however, in other commands, we should not require a connection.

we should show the error for commands: tables, columns, test, profile, explore

we should not require an existing connection for commands: snippets, connect

@edublancas
Copy link
Author

please add unit tests for these cases

@bbeat2782
Copy link

Changed AC based on your comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants