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.

Sign up for GitHub

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

✨ added a --mysql-socket option to connect to MySQL using a Unix socket #129

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

✨ added a --mysql-socket option to connect to MySQL using a Unix socket #129

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
69b6d43
added --mysql-socket option for unix socket path
mackuba Oct 23, 2024
25a9cd7
Merge branch 'master' into socket
techouse Oct 24, 2024
93db8f5
Merge branch 'master' into socket
techouse Oct 26, 2024
602d9af
Merge branch 'master' into socket
techouse Oct 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ Options:
--mysql-password TEXT MySQL password
-h, --mysql-host TEXT MySQL host. Defaults to localhost.
-P, --mysql-port INTEGER MySQL port. Defaults to 3306.
--mysql-socket TEXT Path to MySQL unix socket file.
-S, --skip-ssl Disable MySQL connection encryption.
-i, --mysql-insert-method [DEFAULT|IGNORE|UPDATE]
MySQL insert method. DEFAULT will throw
Expand Down
1 change: 1 addition & 0 deletions docs/README.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ Connection Options
- ``-h, --mysql-host TEXT``: MySQL host. Defaults to localhost.
- ``-P, --mysql-port INTEGER``: MySQL port. Defaults to 3306.
- ``-S, --skip-ssl``: Disable MySQL connection encryption.
- ``--mysql-socket TEXT``: Path to MySQL unix socket file.

Transfer Options
""""""""""""""""
Expand Down
3 changes: 3 additions & 0 deletions src/sqlite3_to_mysql/cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
@click.option("--mysql-password", default=None, help="MySQL password")
@click.option("-h", "--mysql-host", default="localhost", help="MySQL host. Defaults to localhost.")
@click.option("-P", "--mysql-port", type=int, default=3306, help="MySQL port. Defaults to 3306.")
@click.option("--mysql-socket", default=None, help="Path to MySQL unix socket file.")
@click.option("-S", "--skip-ssl", is_flag=True, help="Disable MySQL connection encryption.")
@click.option(
"-i",
Expand Down Expand Up @@ -137,6 +138,7 @@ def cli(
mysql_database: str,
mysql_host: str,
mysql_port: int,
mysql_socket: str,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Update type annotation for mysql_socket parameter

The parameter should be marked as optional since it has a None default value.

-    mysql_socket: str,
+    mysql_socket: t.Optional[str],
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
mysql_socket: str,
mysql_socket: t.Optional[str],

skip_ssl: bool,
mysql_insert_method: str,
mysql_truncate_tables: bool,
Expand Down Expand Up @@ -183,6 +185,7 @@ def cli(
mysql_database=mysql_database,
mysql_host=mysql_host,
mysql_port=mysql_port,
mysql_socket=mysql_socket,
mysql_ssl_disabled=skip_ssl,
mysql_insert_method=mysql_insert_method,
mysql_truncate_tables=mysql_truncate_tables,
Expand Down
3 changes: 3 additions & 0 deletions src/sqlite3_to_mysql/transporter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ def __init__(self, **kwargs: tx.Unpack[SQLite3toMySQLParams]):

self._mysql_port = kwargs.get("mysql_port", 3306) or 3306

self._mysql_socket = str(kwargs.get("mysql_socket")) if kwargs.get("mysql_socket") else None

self._sqlite_tables = kwargs.get("sqlite_tables") or tuple()

self._without_foreign_keys = bool(self._sqlite_tables) or bool(kwargs.get("without_foreign_keys", False))
Expand Down Expand Up @@ -144,6 +146,7 @@ def __init__(self, **kwargs: tx.Unpack[SQLite3toMySQLParams]):
password=self._mysql_password,
host=self._mysql_host,
port=self._mysql_port,
unix_socket=self._mysql_socket,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding socket path validation.

While the implementation is correct, consider validating the socket path's existence before attempting the connection to provide a more user-friendly error message.

+                unix_socket=(
+                    self._mysql_socket
+                    if self._mysql_socket and os.path.exists(self._mysql_socket)
+                    else None
+                ),
-                unix_socket=self._mysql_socket,

This change would help users identify socket path issues before MySQL Connector attempts the connection.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
unix_socket=self._mysql_socket,
unix_socket=(
self._mysql_socket
if self._mysql_socket and os.path.exists(self._mysql_socket)
else None
),

ssl_disabled=self._mysql_ssl_disabled,
use_pure=True,
charset=self._mysql_charset,
Expand Down
2 changes: 2 additions & 0 deletions src/sqlite3_to_mysql/types.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ class SQLite3toMySQLParams(tx.TypedDict):
mysql_password: t.Optional[t.Union[str, bool]]
mysql_host: t.Optional[str]
mysql_port: t.Optional[int]
mysql_socket: t.Optional[str]
mysql_ssl_disabled: t.Optional[bool]
chunk: t.Optional[int]
quiet: t.Optional[bool]
Expand Down Expand Up @@ -49,6 +50,7 @@ class SQLite3toMySQLAttributes:
_mysql_password: t.Optional[str]
_mysql_host: str
_mysql_port: int
_mysql_socket: str
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Type inconsistency between parameter and attribute definitions

The mysql_socket is defined as Optional[str] in SQLite3toMySQLParams but as a required str in SQLite3toMySQLAttributes. This could lead to runtime errors if the socket parameter isn't provided.

Apply this change to maintain consistency:

-    _mysql_socket: str
+    _mysql_socket: t.Optional[str]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
_mysql_socket: str
_mysql_socket: t.Optional[str]

_mysql_ssl_disabled: bool
_chunk_size: t.Optional[int]
_quiet: bool
Expand Down
Loading