Yep. I looked in detail - It does continue after that (It turned out that I
had different, unrelated cherry-picking error - py27 related). So +1 is
still there. Not a blocker.
But It would be nice to fix it as it is quite scary:).
J.
On Sun, Oct 27, 2019 at 5:38 PM Ash Berlin-Taylor <a...@apache.org> wrote:
> Does it fail and terminate the resetdb/migration or just print an error to
> the console? If it's the latter it's been doing that for ages. Yes it
> should be fixed, but it has been doing that for all of 1.10 I think.
>
> When I try it with the dag folder you suggest I see further migrations
> after the error. Don't forget that DagBag loading handles errors, and,
> further migrations run after the error is printed, and `echo $?` reports
> success.
>
> -ash
>
> > On 27 Oct 2019, at 16:33, Jarek Potiuk <jarek.pot...@polidea.com> wrote:
> >
> > I also reconsider my +1 - also because of migrations but for a different
> > reason.
> >
> > During my tests of cherry-picking latest kind changes to v1-10-test I
> have
> > just found another potential problem with migrations - and looking at
> what
> > Fokko reported - we might want to consider reviewing and fixing the
> > migration scripts before 1.10.6.
> >
> > The scenario is a bit involved, but it might cause problems for someone
> > resetting the DB in 1.10.6 with DAGs already in the "dags" folder. Not
> > sure when it was introduced (it has few components introducing the
> > problem), so it might be already in one of the earlier releases.
> >
> > You can reproduce it when you run `airflow resetdb` in the current 1.10.6
> > (I tried with python 2.7 + Postgres) and you
> > have AIRFLOW__CORE__DAGS_FOLDER="S{AIRFLOW_SOURCES}/tests/dags" set
> (which
> > is set in my version of Breeze).
> >
> > The resetdb operation fails in the middle with "cannot import
> > tests/dags/test_clear_subdag.py". I investigated it a bit and I found
> that
> > this is a problem with one of the migration scripts + subdag operator
> > relying on pool_slot relation being present..
> >
> > The problem is in
> *cc1e65623dc7_add_max_tries_column_to_task_instance.py*.
> > The script has this piece of code:
> >
> > if 'task_instance' in tables:
> > # Get current session
> > sessionmaker = sa.orm.sessionmaker()
> > session = sessionmaker(bind=connection)
> > dagbag = DagBag(settings.DAGS_FOLDER)
> >
> > It means that it loads and parses all the dag files during the migration.
> >
> > Then there is this piece of code in *subdag_operator*'s init():
> >
> > # validate that subdag operator and subdag tasks don't have a
> > # pool conflict
> > if self.pool and self.pool != Pool.DEFAULT_POOL_NAME:
> > conflicts = [t for t in subdag.tasks if t.pool == self.pool]
> > if conflicts:
> > # only query for pool conflicts if one may exist
> > pool = (
> > session
> > .query(Pool)
> > .filter(Pool.slots == 1)
> > .filter(Pool.pool == self.pool)
> > .first()
> > )
> >
> > This means that if self.pool is set, then Pool.slots query will be
> > executed. Unfortunately at this time, the slot_pool relation is not
> created
> > yet - because it is created in one of later migrations. Then the query
> > fails with:
> >
> >
> >
> > *psycopg2.ProgrammingError: relation "slot_pool" does not existLINE 2:
> FROM
> > slot_pool*
> >
> > Parsing the test dag fails now because the pool is set to 'default_pool'
> (I
> > guess it used to be None in one of the earlier versions - that's why it
> was
> > not caught before).
> >
> > I am afraid that if someone has a DAG containing SubDag operator they
> will
> > have exactly this problem when running `airflow resetdb`.
> >
> > I am not 100% if this is a blocker but it looks like very close to being
> > one. We could say to users to always remove all the files from airflow's
> > dag folder before running resetdb, but I am afraid this is not really
> nice.
> >
> > J.
> >
> >
> > On Sun, Oct 27, 2019 at 4:48 PM Kaxil Naik <kaxiln...@gmail.com> wrote:
> >
> >> As 2.0 is not released, if migration are broken, we can still fix them
> on
> >> the mater itself.
> >>
> >>
> >>
> >> On Sun, Oct 27, 2019, 15:29 Driesprong, Fokko <fo...@driesprong.frl>
> >> wrote:
> >>
> >>> The problem is that if you upgrade from the 1.10 branch to 2.0, the
> >>> migrations don't work. To probably the migrations in 1.10 and 2.0 have
> >>> diverged somewhere.
> >>>
> >>> The following can be used to reproduce this:
> >>> git pull upstream master # ensure we have latest
> >>> git checkout v-10-stable # 1.10.6rc2
> >>> rm ~/airflow/airflow.db
> >>> airflow initdb
> >>> git checkout master
> >>> airflow db upgrade
> >>>
> >>>
> >>>
> >>> Op zo 27 okt. 2019 om 15:53 schreef Ash Berlin-Taylor <a...@apache.org
> >:
> >>>
> >>>> That PR you mentioned isn't yet in a release, so can we not just edit
> >>> that
> >>>> migration in place?
> >>>>
> >>>> I'm not quite following (I didn't get much sleep) but I don't see how
> >>>> broken code on master affects this release?
> >>>>
> >>>> -a
> >>>>
> >>>>> On 27 Oct 2019, at 14:28, Driesprong, Fokko <fo...@driesprong.frl>
> >>>> wrote:
> >>>>>
> >>>>> I'm having second thoughts on my +1 vote.
> >>>>>
> >>>>> It turns out that the database migrations are broken from 1.10.6 to
> >>>> 2.0.0:
> >>>>>
> >>>>
> >>>
> >>
> https://github.com/apache/airflow/pull/6370/files/c9b9312a60f891475d3072584171c2af56246829#r339287344
> >>>>>
> >>>>> So we either need to release a 1.10.7 and force users to migrate to
> >>> that
> >>>>> version first, before going to 2.0.0, otherwise, it won't work. I've
> >>>> opened
> >>>>> up a PR to fix this for 2.0.0:
> >>>> https://github.com/apache/airflow/pull/6442
> >>>>>
> >>>>> Cheers, Fokko
> >>>>>
> >>>>>
> >>>>> Op za 26 okt. 2019 om 09:23 schreef Jarek Potiuk <
> >>>> jarek.pot...@polidea.com>:
> >>>>>
> >>>>>> +1 (binding)
> >>>>>>
> >>>>>> Tested on Python 3.7/Python 2.7 using local installation + double
> >>>> checked
> >>>>>> sources with RAT licence tool (All good this time!).
> >>>>>>
> >>>>>> Regarding why AIRFLOW-5746 (
> >>> https://github.com/apache/airflow/pull/6434
> >>>> )
> >>>>>> is
> >>>>>> being reverted (or rather being improved on). I don't think it's a
> >>>> blocker
> >>>>>> for this release.
> >>>>>>
> >>>>>> It is just a test change - the tests how they were written, broke
> >> some
> >>>>>> development environments (the airflow resetdb was failing in case
> >>>>>> PYTHONPATH was not specially crafted). The current test does not
> >>> really
> >>>>>> test what it should (passing PYTHONPATH to impersonated tasks) but
> >> the
> >>>>>> functionality (impersonation + path) has not actually changed and it
> >>>> works.
> >>>>>> The test worked fine before the change was added - it's just imports
> >>> in
> >>>>>> tests that have been changed.
> >>>>>>
> >>>>>> Kamil (thanks!) already submitted a fix for that, that solves it
> >>>>>> permanently https://github.com/apache/airflow/pull/6436/ - will
> >>> review
> >>>> and
> >>>>>> merge soon. But it's not a blocker IMHO.
> >>>>>>
> >>>>>> J.
> >>>>>>
> >>>>>> On Sat, Oct 26, 2019 at 2:10 AM Kaxil Naik <kaxiln...@gmail.com>
> >>> wrote:
> >>>>>>
> >>>>>>> +1 (binding)
> >>>>>>>
> >>>>>>> On Sat, Oct 26, 2019 at 12:05 AM Driesprong, Fokko
> >>>> <fo...@driesprong.frl
> >>>>>>>
> >>>>>>> wrote:
> >>>>>>>
> >>>>>>>> +1 binding from my side
> >>>>>>>>
> >>>>>>>> Ran an example DAGs with Docker using Python 3.7.
> >>>>>>>>
> >>>>>>>> We might need to check why AIRFLOW-5746 is being reverted:
> >>>>>>>> https://github.com/apache/airflow/pull/6434
> >>>>>>>>
> >>>>>>>> If there is another RC, I'd like to request to cherry-pick
> >>>>>>>> https://github.com/apache/airflow/pull/6370 onto the 1.10 branch.
> >>>>>>>>
> >>>>>>>> Cheers, Fokko
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Op vr 25 okt. 2019 om 23:04 schreef Ash Berlin-Taylor <
> >>> a...@apache.org
> >>>>>>> :
> >>>>>>>>
> >>>>>>>>> Hey all,
> >>>>>>>>>
> >>>>>>>>> I have cut Airflow 1.10.6 RC2. This email is calling a vote on
> >> the
> >>>>>>>>> release, which will last for 72 hours, until Monday 28, October
> >>> 22nd
> >>>>>> at
> >>>>>>>>> 21:05 UTC.
> >>>>>>>>>
> >>>>>>>>> Consider this my (binding) +1.
> >>>>>>>>>
> >>>>>>>>> Airflow 1.10.6 RC1 is available at: <
> >>>>>>>>> https://dist.apache.org/repos/dist/dev/airflow/1.10.6rc2/>
> >>>>>>>>>
> >>>>>>>>> *apache-airflow-1.10.6rc2-source.tar.gz* is a source release that
> >>>>>> comes
> >>>>>>>>> with INSTALL instructions.
> >>>>>>>>> *apache-airflow-1.10.6rc2-bin.tar.gz* is the binary Python
> >> "sdist"
> >>>>>>>> release.
> >>>>>>>>> *apache_airflow-1.10.6rc2-py2.py3-none-any.whl* is the binary
> >>> Python
> >>>>>>>>> "wheel" release.
> >>>>>>>>>
> >>>>>>>>> Public keys are available at: <
> >>>>>>>>> https://dist.apache.org/repos/dist/release/airflow/KEYS>
> >>>>>>>>>
> >>>>>>>>> As per normal the rc1 is available for testing from PyPi.
> >>>>>>>>>
> >>>>>>>>> Only votes from PMC members are binding, but members of the
> >>> community
> >>>>>>> are
> >>>>>>>>> encouraged to test the release and vote with "(non-binding)".
> >>>>>>>>>
> >>>>>>>>> Please note that the version number excludes the `rcX` string, so
> >>>>>> it's
> >>>>>>>> now
> >>>>>>>>> simply 1.10.6. This will allow us to rename the artifact without
> >>>>>>>> modifying
> >>>>>>>>> the artifact checksums when we actually release.
> >>>>>>>>>
> >>>>>>>>> The changes since RC1 are to fix License issues, ensure tests are
> >>>>>>> running
> >>>>>>>>> on Py2 (they weren't, but the only py3 bits that crept in were in
> >>> the
> >>>>>>>> test
> >>>>>>>>> files luckily.).
> >>>>>>>>>
> >>>>>>>>> Changelog since 1.10.6rc1:
> >>>>>>>>>
> >>>>>>>>> * 73bf71835 [AIRFLOW-XXX] Update date in changelog [Ash
> >>>>>> Berlin-Taylor]
> >>>>>>>>> * 143b43151 [AIRFLOW-5750] Licence check is done also for
> >>>>>>> non-executable
> >>>>>>>>> .sh (#6425) [Jarek Potiuk]
> >>>>>>>>> * 544f2b336 [AIRFLOW-5754] Improved RAT checking (#6429) [Jarek
> >>>>>> Potiuk]
> >>>>>>>>> * 7904669ca [AIRFLOW-5755] Fixed most problems with py27 [Jarek
> >>>>>> Potiuk]
> >>>>>>>>> * d601752c4 [AIRFLOW-5748] Remove python auto-detection (#6423)
> >>>>>> [Jarek
> >>>>>>>>> Potiuk]
> >>>>>>>>> * 71e20417f [AIRFLOW-5746] Fix problems with static checks
> >> (#6420)
> >>>>>>> [Jarek
> >>>>>>>>> Potiuk]
> >>>>>>>>> * 7a6adad60 [AIRFLOW-5746] move FakeDateTime into the only place
> >> it
> >>>>>> is
> >>>>>>>>> used (#6416) [Michael R. Crusoe]
> >>>>>>>>> * e30fb85ca [AIRFLOW-5745] Breeze complete has now licence
> >> (#6415)
> >>>>>>> [Jarek
> >>>>>>>>> Potiuk]
> >>>>>>>>>
> >>>>>>>>> Files changes since rc1:
> >>>>>>>>>
> >>>>>>>>> airflow ❯ git diff --stat 1.10.6rc1...1.10.6rc2
> >>>>>>>>> .pre-commit-config.yaml | 1 -
> >>>>>>>>> .rat-excludes | 1 +
> >>>>>>>>> .travis.yml | 47
> >>>>>>>>> +++++++++++++++++++++++++++++++++++++++--------
> >>>>>>>>> CHANGELOG.txt | 2 +-
> >>>>>>>>> Dockerfile-checklicence | 2 +-
> >>>>>>>>> airflow/models/dag.py | 5 ++++-
> >>>>>>>>> breeze | 1 +
> >>>>>>>>> breeze-complete | 17
> >> +++++++++++++++++
> >>>>>>>>> common/_autodetect_variables.sh | 49
> >>>>>>>>> +++++++++++++------------------------------------
> >>>>>>>>> files/x | 0
> >>>>>>>>> files/y | 0
> >>>>>>>>> scripts/ci/_utils.sh | 16
> >> ++++++++++++++--
> >>>>>>>>> scripts/ci/ci_check_license.sh | 1 +
> >>>>>>>>> scripts/ci/ci_run_airflow_testing.sh | 2 +-
> >>>>>>>>> scripts/ci/in_container/run_check_licence.sh | 2 +-
> >>>>>>>>> tests/contrib/hooks/test_ssh_hook.py | 3 ++-
> >>>>>>>>> tests/dags/test_impersonation_custom.py | 12 +++++++++++-
> >>>>>>>>> tests/models/test_baseoperator.py | 9 ++++-----
> >>>>>>>>> tests/models/test_dag.py | 4 ++--
> >>>>>>>>> tests/test_sentry.py | 2 +-
> >>>>>>>>> tests/test_utils/fake_datetime.py | 29
> >>>>>>>>> -----------------------------
> >>>>>>>>> tests/test_utils/system_tests_class.py | 6 +++++-
> >>>>>>>>> tests/www/test_views.py | 6 +++---
> >>>>>>>>> 23 files changed, 122 insertions(+), 95 deletions(-)
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>>
> >>>>>> --
> >>>>>>
> >>>>>> Jarek Potiuk
> >>>>>> Polidea <https://www.polidea.com/> | Principal Software Engineer
> >>>>>>
> >>>>>> M: +48 660 796 129 <+48660796129>
> >>>>>> [image: Polidea] <https://www.polidea.com/>
> >>>>>>
> >>>>
> >>>>
> >>>
> >>
> >
> >
> > --
> >
> > Jarek Potiuk
> > Polidea <https://www.polidea.com/> | Principal Software Engineer
> >
> > M: +48 660 796 129 <+48660796129>
> > [image: Polidea] <https://www.polidea.com/>
>
>
--
Jarek Potiuk
Polidea <https://www.polidea.com/> | Principal Software Engineer
M: +48 660 796 129 <+48660796129>
[image: Polidea] <https://www.polidea.com/>
