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

update task_parameter and set value to TEXT type #2241

Closed
wants to merge 3 commits into from

Conversation

jimexist
Copy link
Contributor

fixes #2240

Description

Motivation and Context

Have you tested this? If so, how?

@dlstadther
Copy link
Collaborator

I don't have any experience with this portion of Luigi. @erikbern @stephenpascoe could y'all take a look given your past contributions to this module? Thanks!

@jimexist
Copy link
Contributor Author

jimexist commented Sep 27, 2017

thanks for bringing up the conversation. travis failed because i used postgres syntax but the sqlite didn't support.

another thing that i did also want to confirm is that task_id index was not a unique index but it was used like one, i.e. fetching by task_id is assumed to return one row. this was not enforced in database level - but i think it should be.

@kwilcox
Copy link
Contributor

kwilcox commented Apr 24, 2018

Just recently hit this issue in production when a new task went over the existing 256 character limit. I've tested this PR and migrated an existing postgres database just fine. However, based on some existing issues (#2370), at least one person is using MS SQL and many people use SQLite as their task stores so we at least need to support those dialects. We can just else here https://github.com/Jimexist/luigi/blob/64e861dbfb4f7d851cb3ae6c29e0b54e3dcaf8f5/luigi/db_task_history.py#L261 instead of a else if and cover most of the cases. Oracle will need a specific check since its ALTER TABLE syntax after 10g is a bit different when modifying columns.

@jimexist
Copy link
Contributor Author

jimexist commented May 5, 2018

thanks for fixing this

@jimexist jimexist closed this May 5, 2018
@jimexist jimexist deleted the patch-1 branch May 5, 2018 05:27
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.

luigi parameter value column is set to limit size of 256
3 participants