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

[AIRFLOW-4961] Set TaskFail.duration as int match DB schema column type #5593

Merged

Conversation

fahran
Copy link
Contributor

@fahran fahran commented Jul 15, 2019

When writing a task failure record, convert 'duration' decimal
value -> an int before persistence, to remove reliance on the
database doing this automatically and gracefully.


Make sure you have checked all steps below.

Jira

Description

  • [✅] Here are some details about my PR, including screenshots of any UI changes:
    I raised https://issues.apache.org/jira/browse/AIRFLOW-4961 earlier today, to cover a minor issue we encountered whilst running airflow using CockroachDB. That ticket covers the full explanation of this PR.

Tests

  • [ 🤔] My PR adds the following unit tests OR does not need testing for this extremely good reason:
    There aren't currently unit tests for testfail.py (that's not to say there shouldn't be), and an integration test would pass with the present code anyway, as Postgres' error handling masks the issue.
    It's also a pretty trivial change. Happy to add a test for testfail.py if we feel it's needed though.

Commits

  • [ ✅] My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":
    1. Subject is separated from body by a blank line
    2. Subject is limited to 50 characters (not including Jira issue reference)
    3. Subject does not end with a period
    4. Subject uses the imperative mood ("add", not "adding")
    5. Body wraps at 72 characters
    6. Body explains "what" and "why", not "how"

Documentation

  • [ ✅] In case of new functionality, my PR adds documentation that describes how to use it.
    • All the public functions and the classes in the PR contain docstrings that explain what it does
    • If you implement backwards incompatible changes, please leave a note in the Updating.md so we can assign it to a appropriate release

Code Quality

  • [ ✅ ] Passes flake8

When writing a task failure record, convert 'duration' decimal
value -> an int before persistence, to remove reliance on the
database doing this automatically and gracefully.
@ashb
Copy link
Member

ashb commented Jul 15, 2019

To replicate my comment from the Jira: should we change the column type instead? sub-second accuracy on the duration of an airflow task probably isn't all that useful is it?

@ashb ashb changed the title [AIRFLOW-4961] Prevent cast when writing TaskFail model to DB [AIRFLOW-4961] Set TaskFail.duration as int match DB schema column type Jul 30, 2019
@ashb ashb merged commit 7d02a9d into apache:master Jul 30, 2019
ashb pushed a commit to ashb/airflow that referenced this pull request Jul 30, 2019
… type (apache#5593)

When writing a task failure record, convert 'duration' decimal
value -> an int before persistence, to remove reliance on the
database doing this automatically and gracefully.

(cherry picked from commit 7d02a9d)
ashb pushed a commit that referenced this pull request Jul 30, 2019
… type (#5593)

When writing a task failure record, convert 'duration' decimal
value -> an int before persistence, to remove reliance on the
database doing this automatically and gracefully.

(cherry picked from commit 7d02a9d)
dharamsk pushed a commit to postmates/airflow that referenced this pull request Aug 8, 2019
… type (apache#5593)

When writing a task failure record, convert 'duration' decimal
value -> an int before persistence, to remove reliance on the
database doing this automatically and gracefully.

(cherry picked from commit 7d02a9d)
@fahran fahran deleted the bugfix/AIRFLOW-4961-task-failure-sql-error branch November 14, 2019 23:51
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.

None yet

2 participants