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

Enhancement request: move pyDAL to local import #234

Closed
colesmj opened this issue Mar 9, 2024 · 4 comments
Closed

Enhancement request: move pyDAL to local import #234

colesmj opened this issue Mar 9, 2024 · 4 comments
Assignees

Comments

@colesmj
Copy link
Collaborator

colesmj commented Mar 9, 2024

pyDAL is used for only 1 purpose within 1 function - pytm.py:1175 SQLDump()
For readability and to reduce pre-installation requirements for likely most users, consider making the import of pyDAL currently on pytm.py:24 be local to SQLDump(). This is allowed by Python and means if no one uses SQLDump() pyDAL is never loaded; otherwise it is a requirement and always loaded even if never used.

@izar
Copy link
Collaborator

izar commented Mar 13, 2024

hm, but wouldn't that cause a fail if someone started slow and didn't use the sql functionality, but then did so? we would be banking on the idea that they know how to deal with an import error in Python.

@raphaelahrens
Copy link
Contributor

You can catch the failed import error and print guidance on how to proceed to stdout.

try:
    import pyDAL
except ImportError:
   print("please do ....")

@izar
Copy link
Collaborator

izar commented Mar 13, 2024

... and I just learned a new one. This day already paid off, thanks!
In that case, PR away!

raphaelahrens added a commit to raphaelahrens/pytm that referenced this issue Mar 13, 2024
In OWASP#234 @colesmj suggested to move the import of pydal into the sqldumb
function.
This commit does this and if the import fails raises an UIError with an
explanation on how to proceed.
The text is just a first draft.
raphaelahrens added a commit to raphaelahrens/pytm that referenced this issue Mar 13, 2024
In OWASP#234 @colesmj suggested to move the import of pydal into the sqlDumb
function.
This commit does this and if the import fails raises an UIError with an
explanation on how to proceed.
The text is just a first draft.

To move the import the function get_table was also moved inside the
sqlDump function.
raphaelahrens added a commit to raphaelahrens/pytm that referenced this issue Mar 13, 2024
In OWASP#234 @colesmj suggested to move the import of pydal into the sqlDumb
function.
This commit does this and if the import fails raises an UIError with an
explanation on how to proceed.
The text is just a first draft.

To move the import the function get_table was also moved inside the
sqlDump function.
izar added a commit that referenced this issue Mar 13, 2024
@raphaelahrens
Copy link
Contributor

I think this can be closed now.

@izar izar closed this as completed Mar 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants