Skip to content

Conversation

@kaxil
Copy link
Member

@kaxil kaxil commented Oct 6, 2020

There is no reason to limit tenacity. One of my upcoming PRs (or a commit in scheduler-ha PR) will use a new feature available in the newer version.

One of the reasons for creating a separate PR for this is so that constraints are updated and when I create new PR (or merge to scheduler-ha PR) it will run against a newer version of tenacity because of the upgraded constraints, without it, it will error.


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

@kaxil kaxil requested review from ashb and dimberman October 6, 2020 17:34
@ryw ryw self-requested a review October 6, 2020 17:42
Copy link
Member

@ryw ryw left a comment

Choose a reason for hiding this comment

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

+1

@potiuk
Copy link
Member

potiuk commented Oct 6, 2020

@kaxil -> I would like to check if this works - do not merge it please before I take a look. I have a feeling we have some problem with upgrading constraints and I would like to have closer look tomorrow on it.

Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

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

Just want to take a look at this before it gets merged.

@ashb
Copy link
Member

ashb commented Oct 6, 2020

why did we limit it in the first place? something must have broken with 5.2?

@potiuk
Copy link
Member

potiuk commented Oct 6, 2020

I think @kaxil wants to update constraints. But I am afraid it will not work this way. I will take a look at this thing tomorrow.

@kaxil
Copy link
Member Author

kaxil commented Oct 6, 2020

I think @kaxil wants to update constraints. But I am afraid it will not work this way. I will take a look at this thing tomorrow.

@potiuk No no my intention is to increase the limit too, constraints is just the by-product which will help in the next PR that will use the feature from latest version

@kaxil
Copy link
Member Author

kaxil commented Oct 6, 2020

why did we limit it in the first place? something must have broken with 5.2?

@ashb

No there is no reason it was pinned, apparently, I did that one too: 926aa1b

@kaxil
Copy link
Member Author

kaxil commented Oct 6, 2020

Just to be clear, I want to update requirements not just constraints. Constraints is the by-product. I am familiar with the current CI problem regarding constraints.

@kaxil kaxil merged commit 4af7804 into apache:master Oct 6, 2020
@kaxil kaxil deleted the bump-tenacity branch October 6, 2020 20:52
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