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

Bump croniter from <1.1 to <1.2 #20489

Merged
merged 2 commits into from Dec 29, 2021
Merged

Bump croniter from <1.1 to <1.2 #20489

merged 2 commits into from Dec 29, 2021

Conversation

ashwin153
Copy link
Contributor

This minor release makes cron validation stricter. https://pypi.org/project/croniter/1.1.0/#id1

Enforce validation for month=1. Before this release we used to support month=0 and it was silently glided to month=1 to support having both day in month in 4th field when it came to have 6fields cron forms (second repeat). It will now raises a CroniterBadDateError. See kiorky/croniter#6 [kiorky]

Copy link
Member

@uranusjr uranusjr left a comment

Choose a reason for hiding this comment

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

Maybe we should bump all the way to <2.0

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Dec 24, 2021
@github-actions
Copy link

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

setup.cfg Outdated Show resolved Hide resolved
@potiuk
Copy link
Member

potiuk commented Dec 28, 2021

Maybe we should bump all the way to <2.0

Why don't we remove any upper limit ? This should be our default action. We should limit something only upper-bound only if we know it does not work. We cannot rely on SemVer and we have constraints that protect us.

I just added a commit to see if tests pass.

@potiuk
Copy link
Member

potiuk commented Dec 29, 2021

Looks good. Merging :)

@potiuk potiuk merged commit 8a03a50 into apache:main Dec 29, 2021
@potiuk potiuk added this to the Airflow 2.2.4 milestone Jan 22, 2022
potiuk pushed a commit that referenced this pull request Jan 22, 2022
(cherry picked from commit 8a03a50)
@jedcunningham jedcunningham added the type:bug-fix Changelog: Bug Fixes label Jan 25, 2022
jedcunningham pushed a commit that referenced this pull request Jan 27, 2022
(cherry picked from commit 8a03a50)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
full tests needed We need to run full set of tests for this PR to merge type:bug-fix Changelog: Bug Fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants