nakagami / pyfirebirdsql

Python DBAPI module for FirebirdSQL
BSD 2-Clause "Simplified" License
71 stars 29 forks source link

The `passlib` dependency is missing from the requirements #107

Open DeadNews opened 4 months ago

DeadNews commented 4 months ago

No longer works without passlib.

https://github.com/nakagami/pyfirebirdsql/compare/REL_1_2_5...REL_1_2_6#diff-d78baff5db7c62238ccfd1be0dd570c7dc8cde9f574a32367d5c095a698ba998R75 https://github.com/nakagami/pyfirebirdsql/compare/REL_1_2_5...REL_1_2_6#diff-4d4623fa054c77b9ad2dd3b911da2a95b226089c97734658a7f788c82fbcfe23R38

  File "C:\Users\user\.local\pipx\venvs\venv\Lib\site-packages\firebirdsql\__init__.py", line 94, in connect
    conn._initialize_socket()
  File "C:\Users\user\.local\pipx\venvs\venv\Lib\site-packages\firebirdsql\fbcore.py", line 989, in _initialize_socket
    self._op_attach(self.timezone)
  File "C:\Users\user\.local\pipx\venvs\venv\Lib\site-packages\firebirdsql\wireprotocol.py", line 493, in _op_attach
    enc_pass = get_crypt(self.password)
               ^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\user\.local\pipx\venvs\venv\Lib\site-packages\firebirdsql\wireprotocol.py", line 75, in get_crypt
    from passlib.hash import des_crypt
ModuleNotFoundError: No module named 'passlib'

ref: https://github.com/nakagami/pyfirebirdsql/pull/105

nakagami commented 4 months ago

passlib is not always required

When not required

When required

In preparation for the upcoming 3.13 release, passlib is imported before the crypt module when authenticating with Legacy_Auth, and passlib is also used for testing. However, from the past to the future, it does not always work without passlib, rather, passlib is only used in some combinations.

As of Firebird 5.0, it seems that authentication with Legacy_Auth is no longer possible, so it makes no sense to install passlib in Firebird 5.0.

I don't want to list extras_require for a limited users.

I want someone to know that they only need to install passlib if they get an import error.

DeadNews commented 4 months ago

This is boolean logic. Or the dependency is used in the executable code and specified in the requirements. Or the dependency is not used in the executable code.

What is installed in the virtual environment is determined by the list of dependencies. It must be complete.

The dependency list can be restricted by Python versions, platforms, etc¹. You cannot restrict it by Firebird version, Python knows nothing about that. The dependency must be specified even if it is not used for all versions of Firebird. Or this code should only be executed for certain versions of Firebird.

I don't see any problem with adding a 525.6 kb pure python² dependency.

And yes, in 90 days Python 3.13 will be released, and sooner or later all users will need passlib.


¹ In general, all these restrictions are needed for dependencies that only work on certain Python versions, platforms, etc. ² It is guaranteed to work on any system that Python works on.

DeadNews commented 4 months ago

https://github.com/nakagami/pyfirebirdsql/blob/e827dba36805816ea2b1913de5aaa23fc8e87123/firebirdsql/wireprotocol.py#L68-L77

If you ask me, I would just use passlib.

from passlib.hash import des_crypt 

def get_crypt(plain): 
    return des_crypt.using(salt='9z').hash(plain)[2:] 

crypt doesn't support all systems anyway and will be removed in 90 days.

https://docs.python.org/3/library/crypt.html

Deprecated since version 3.11, will be removed in version 3.13: The crypt module is deprecated (see PEP 594 for details and alternatives). The hashlib module is a potential replacement for certain use cases. The passlib package can replace all use cases of this module.