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

Scope SQLA session so object lifespans rendering #3153

Merged
merged 1 commit into from Apr 28, 2022

Conversation

rjcortese
Copy link
Contributor

Description

This is a resubmittal of the stale PR #3058 that fixes issue #3006.
That commit is now on top of the master branch.

Motivation and Context

Fixes #3006.
Original author was unable to proceed with PR.

Have you tested this? If so, how?

I have not... will make modifications based on how the CI responds.

@rjcortese rjcortese requested review from dlstadther and a team as code owners March 27, 2022 23:38
@rjcortese
Copy link
Contributor Author

Hmm so docs won't build...
Sphinx 1.4.9 wants to import something that has been removed from Jinja2 (or rather the name has changed... contextfunction is now pass_context).
Would it be alright to try to increase the Sphinx version?

luigi/tox.ini

Line 135 in e20e430

Sphinx>=1.4.4,<1.5

The most recent release is 4.5.0

@dlstadther
Copy link
Collaborator

Yes, sorry. I fixed the build issues this weekend, but am waiting on an approval so that I can merge it.

If you'd like, you can try to give it an approval. I'm not sure if i can merge with any approval, or if it requires a maintainer approval. Although I have merge permissions, I cannot approve my own PR nor merge without an approval.

Once that unittest fix goes through, i have a few PR reviews to catch up on.

@rjcortese
Copy link
Contributor Author

Sorry, but it seems that PR of yours requires review by a reviewer with write access. I do not have write access :(

@dlstadther
Copy link
Collaborator

Sorry, but it seems that PR of yours requires review by a reviewer with write access. I do not have write access :(

That's ok. I appreciate you trying.

@dlstadther dlstadther merged commit 5a69669 into spotify:master Apr 28, 2022
@rjcortese rjcortese deleted the issue#3006-stalepr#3058 branch April 28, 2022 12:41
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.

Enabling task history schedule
3 participants