-
Notifications
You must be signed in to change notification settings - Fork 85
Amjith/sqlean optional #221
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
Conversation
|
||
sqlite3.extensions.enable_all() | ||
except ImportError: | ||
import sqlite3 |
Check notice
Code scanning / CodeQL
Module is imported with 'import' and 'import from' Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 20 days ago
To fix the issue:
- Remove the
from sqlite3 import OperationalError
statement on line 12. - Replace any direct usage of
OperationalError
withsqlite3.OperationalError
in the code. - Ensure that the functionality remains unchanged by testing the code after the fix.
This approach adheres to the recommendation and eliminates the redundant import while maintaining clarity and consistency.
-
Copy modified line R7
@@ -6,3 +6,3 @@ | ||
import sqlean as sqlite3 | ||
from sqlean import OperationalError | ||
OperationalError = sqlite3.OperationalError | ||
|
||
@@ -11,3 +11,2 @@ | ||
import sqlite3 | ||
from sqlite3 import OperationalError | ||
from litecli.packages.special.utils import check_if_sqlitedotcommand |
try: | ||
from sqlean import OperationalError, sqlite_version | ||
except ImportError: | ||
from sqlite3 import OperationalError, sqlite_version |
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.
The import looks fine and should handle the case when sqlean is missing👍🏾
|
||
sqlite3.extensions.enable_all() | ||
except ImportError: | ||
import sqlite3 |
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.
This looks good to cover missing sqlean.
@@ -27,14 +27,14 @@ def list_tables(cur, arg=None, arg_type=PARSED_QUERY, verbose=False): | |||
args = ("{0}%".format(arg),) | |||
query = """ | |||
SELECT name FROM sqlite_master | |||
WHERE type IN ('table','view') AND name LIKE ? AND name NOT LIKE 'sqlite_%' | |||
WHERE type IN ('table','view') AND name LIKE ? AND name NOT LIKE 'sqlite_%' AND name NOT LIKE 'sqlean_define' |
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.
Nit: We can be a bit more generic, like sqlean_%
. Not blocking the PR though
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.
Good point. Done!
We are good to merge the PR! |
@kracekumar This is just a draft looking for feedback.
Description
Make
sqlean
an optional dependency.Test with and without sqlean.
Update docs to have
sqlean
as part of the install command.