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

confusing error when executing %sqlcmd #262

Closed
edublancas opened this issue Mar 17, 2023 · 8 comments · Fixed by #271
Closed

confusing error when executing %sqlcmd #262

edublancas opened this issue Mar 17, 2023 · 8 comments · Fixed by #271
Assignees
Labels
good first issue Good for newcomers

Comments

@edublancas
Copy link

When executing %sqlcmd, I get this:

Screenshot 2023-03-16 at 6 44 33 p m

But the error is unclear. Instead, we should throw an UsageError and tell the user what are the available commands, along with some examples

@raghavSharmaCode
Copy link

Request to assign this issue (#262)

@raghavSharmaCode
Copy link

Hi Ido/ Eduardo,

I would like to know the control flow of the above command '%sqlcmd'.

I have forked the repo and created a new branch 'dev-262'. I changed the file magic_cmd.py to debug the issue. The issue occurs at line 44. So after line 43, I added some print statements to check the split of the line argument.

However, when I committed the file and tried to install the repo as a pip package using:

(pip install git+https://github.com/raghavSharmaCode/jupysql.git@dev-262)

It did not reflect the changes that I was expecting when I ran the '%sqlcmd' command in a test .ipynb file.

@raghavSharmaCode
Copy link

Hence I would like to learn about the OOPS implementation of this package so that we are able to debug and resolve the issue on a low-level design basis.

@edublancas
Copy link
Author

edublancas commented Mar 18, 2023

did you setup with the invoke setup command? Once you fork and clone the repo, you can run:

invoke setup

This will install JupySQL in editable mode, meaning all changes should reflect (if you're using Jupyter, you need to enable autoreload or restart the kernel, though)

To check if JupySQL is correctly installed in the current environment:

In [1]: import sql; sql
Out[1]: <module 'sql' from '/Users/eduardo/dev/jupysql/src/sql/__init__.py'>

You can see that sql prints the path where I cloned the repo, you should see something similar

@raghavSharmaCode
Copy link

Hi Eduardo,

Please confirm if the below image explains what you asked for.

Ploomber issue #262

The latest code is in the dev-262 repo.

Please confirm if I should raise a pull request for you to verify.

Thanks and regards,
Raghav

@edublancas
Copy link
Author

Looks good!

Some feebdback:

If the user passes

%sqlcmd

The error should say:

Missing argument for %sqlcmd
Valid commands are: tables, columns

If they pass a wrong argument:

%sqlcmd stuff

The error should say:

stuff is not a valid argument for %sqlcmd
Valid commands are: tables, columns

@raghavSharmaCode
Copy link

As requested,

#262-final

Also, I would like to know the significance of the default line argument as an empty string. An empty string is a use case but how much is it relevant as the default argument?

I will be raising a pull request for you to verify the changes. Looking forward to your response.

Best,
Raghav

@edublancas
Copy link
Author

for reference, the PR is: #271

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

Successfully merging a pull request may close this issue.

3 participants