Skip to content

Conversation

@msumit
Copy link
Contributor

@msumit msumit commented Nov 24, 2019

Make sure you have checked all steps below.

Jira

Description

  • Adding an option in sensors to do exponential backoff before poke or schedule again.

Tests

  • Added a couple of unit tests around the new function

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

@codecov-io
Copy link

codecov-io commented Nov 24, 2019

Codecov Report

Merging #6654 into master will decrease coverage by 0.32%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #6654      +/-   ##
=========================================
- Coverage   83.82%   83.5%   -0.33%     
=========================================
  Files         672     672              
  Lines       37560   37576      +16     
=========================================
- Hits        31483   31376     -107     
- Misses       6077    6200     +123
Impacted Files Coverage Δ
airflow/sensors/base_sensor_operator.py 98.57% <100%> (+0.42%) ⬆️
airflow/kubernetes/volume_mount.py 44.44% <0%> (-55.56%) ⬇️
airflow/kubernetes/volume.py 52.94% <0%> (-47.06%) ⬇️
airflow/kubernetes/pod_launcher.py 45.25% <0%> (-46.72%) ⬇️
airflow/kubernetes/refresh_config.py 50.98% <0%> (-23.53%) ⬇️
...rflow/contrib/operators/kubernetes_pod_operator.py 78.2% <0%> (-20.52%) ⬇️
airflow/configuration.py 89.13% <0%> (-3.63%) ⬇️
airflow/utils/dag_processing.py 57.99% <0%> (-0.5%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update db4be19...a1eab04. Read the comment docs.

@msumit msumit requested review from Fokko, aoen, ashb and kaxil November 25, 2019 05:22
interval1 = sensor._get_next_poke_interval(started_at, 1)
interval2 = sensor._get_next_poke_interval(started_at, 2)

self.assertTrue(interval2 >= sensor.poke_interval >= interval1)
Copy link
Member

Choose a reason for hiding this comment

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

Here too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

problem is that this method calculates sha1, which is based upon the started_at, which is relative to the current time.

Copy link
Member

Choose a reason for hiding this comment

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

In which case use mocking to make timezone.utcnow return a known constant value in the tests.

Or if the "jitter" caused by this is small then

self.assertGreaterThanEqual(interval1, 5)
self.assertLessThanEqual(interval1, 5.5)

self.assertGreaterThanEqual(interval2, 6)
self.assertLessThanEqual(interval2, 7)

Those values are not right, but this is the sort of thing I'd like to see.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mocked timezone

Copy link
Member

Choose a reason for hiding this comment

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

You mocked the timezone, but didn't add the extra tests that I asked for. Please could you add them?

@kaxil kaxil changed the title AIRFLOW-6055: Option for exponential backoff in Sensors [AIRFLOW-6055] Option for exponential backoff in Sensors Nov 25, 2019
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.

4 participants