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

Fix pymssql import #3252

Merged
merged 3 commits into from
Sep 22, 2023
Merged

Fix pymssql import #3252

merged 3 commits into from
Sep 22, 2023

Conversation

grihabor
Copy link
Contributor

@grihabor grihabor commented Sep 13, 2023

Description

Fix import error in luigi.contrib.mssqldb

Motivation and Context

I'm getting this error even though I have pymssql installed:

Traceback (most recent call last):
  File "/opt/luigi/lib/python3.10/site-packages/luigi/worker.py", line 434, in check_complete
    is_complete = check_complete_cached(task, completion_cache)
  File "/opt/luigi/lib/python3.10/site-packages/luigi/worker.py", line 419, in check_complete_cached
    is_complete = task.complete()
  File "/opt/luigi/lib/python3.10/site-packages/luigi/task.py", line 926, in complete
    return all(r.complete() for r in flatten(self.requires()))
  File "/opt/luigi/lib/python3.10/site-packages/luigi/task.py", line 926, in <genexpr>
    return all(r.complete() for r in flatten(self.requires()))
  File "/opt/luigi/lib/python3.10/site-packages/luigi/task.py", line 594, in complete
    return all(map(lambda output: output.exists(), outputs))
  File "/opt/luigi/lib/python3.10/site-packages/luigi/task.py", line 594, in <lambda>
    return all(map(lambda output: output.exists(), outputs))
  File "/opt/luigi/lib/python3.10/site-packages/luigi/contrib/mssqldb.py", line 104, in exists
    connection = self.connect()
  File "/opt/luigi/lib/python3.10/site-packages/luigi/contrib/mssqldb.py", line 123, in connect
    connection = _mssql.connect(user=self.user,
NameError: name '_mssql' is not defined

@grihabor grihabor requested review from dlstadther and a team as code owners September 13, 2023 16:41
@grihabor grihabor changed the title import pymssql Fix pymssql import Sep 13, 2023
Copy link
Collaborator

@dlstadther dlstadther left a comment

Choose a reason for hiding this comment

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

This file does not appear to have any unittests. In its place, can you confirm that after installing pymssql and instantiating content from this file (with your changes), it works as expected?

The changes look like they should be good, but I cannot easily confirm that all remaining imports and references are still correct.

@grihabor
Copy link
Contributor Author

@dlstadther Yes, we're using this patch in production

@dlstadther dlstadther merged commit ef9952a into spotify:master Sep 22, 2023
33 of 35 checks passed
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

Successfully merging this pull request may close these issues.

2 participants