Improvement of db_helper for sqlite support #52
Conversation
|
@suyashgautam kindly review the changes whenever you get the time |
src/server/db/db_helper.py
Outdated
| @@ -438,9 +438,15 @@ async def validate_sql(self, db_name, query): | |||
| # return False | |||
|
|
|||
| class sqlite_database(Database): | |||
| def __init__(self): | |||
| self.temp_db_folder = "temporary_db" | |||
There was a problem hiding this comment.
Dont call the folder as temporary_db.
Make the directory like this.
There is a folder called files at the root where we store all the sql files. I would suggest create a fodler sqlite_db inside the files folder and then save the db files in that fodler
src/server/db/db_helper.py
Outdated
| def __init__(self): | ||
| self.temp_db_folder = "temporary_db" | ||
| if not os.path.exists(self.temp_db_folder): | ||
| os.makedirs(self.temp_db_folder) |
There was a problem hiding this comment.
Dont use temp_db_folder as variable name. Give it a more descriptive name
src/server/db/db_helper.py
Outdated
| async def get_connection(self): | ||
| try: | ||
| con = sqlite3.connect("temp.db") | ||
| db_file_path = os.path.join(self.temp_db_folder, "temp.db") |
There was a problem hiding this comment.
I believe you are not using this function anywhere so dont use temp.db to create a connection. Dont define this function if you are not using it
There was a problem hiding this comment.
I have removed it and added a pass statement, because the function itself may be called at times, but it does not do anything, completely removing it may cause issues in app.py itself.
src/server/db/db_helper.py
Outdated
| @@ -488,7 +496,8 @@ async def create_schema_in_db(self, db_name, schema): | |||
|
|
|||
| async def get_tables_from_schema_id(self, schema_id): | |||
| try: | |||
| con = sqlite3.connect(f'{db_name}.db') | |||
| db_file_path = os.path.join(self.temp_db_folder, f'{db_name}.db') | |||
There was a problem hiding this comment.
you are getting the db name as schema_id but you are using db_name as the file name. Change it schema_id
src/server/db/db_helper.py
Outdated
| @@ -500,7 +509,8 @@ async def get_tables_from_schema_id(self, schema_id): | |||
|
|
|||
| async def get_table_info(self, db_name, table_name): | |||
| try: | |||
| con = sqlite3.connect(f'{db_name}.db') | |||
| db_file_path = os.path.join(self.temp_db_folder, f'{db_name}.db') | |||
There was a problem hiding this comment.
Use a more descriptive name for temp_db_folder variable
There was a problem hiding this comment.
changed it to sqlite_db_folder
|
Merge this code as part of #53 Hence closing this ticket |
|
Merge this code as part of #53 Hence closing this ticket |
Earlier db_helper for sqlite support was storing the temporary db in the root directory. After this PR it will store the temporary db files in a temporary directory that can be deleted later.
Regex has also been improved in create_schema_for_db to take care of the case when semi-colon (;) may be present inside the query.