search

apache / airflow

Apache Airflow - A platform to programmatically author, schedule, and monitor workflows
https://airflow.apache.org/
Apache License 2.0
36.73k stars 14.22k forks source link

Create an `airflow upgrade-check` command in 1.10 to ease upgrade path to 2.0. #8765

Closed ashb closed 3 years ago

ashb commented 4 years ago

To make it easier for users to upgrade from 1.10 to 2.0 (when it eventually comes out) we should create a single upgrade-check command in 1.10 that checks the following things. We could also have a mode that makes some of these changes in place (with confirmation from user) to automate it.

Rules

Major breaking changes:

Config related changes:

Import changes:

How to guide

To implement a new rule we had to create a class that inherits from airflow.upgrade.rules.base_rule.BaseRule. It will be auto-registered and used by airflow upgrade-check command. The custom rule class has to have title, description properties and should implement check method which returns a list of error messages in case of incompatibility.

For example: https://github.com/apache/airflow/blob/ea36166961ca35fc51ddc262ec245590c3e236fb/airflow/upgrade/rules/conn_type_is_not_nullable.py#L25-L42

Remeber to open the PR against v1-10-test branch.

turbaszek commented 4 years ago

I screened the UPDATING.md for "breaking" changes that can be check / communicated. I selected those explicitly saying "breaking" or sounding like that. In general, we can split them into three categories:

I ended up with a list of 59 of changes:

 'BaseOperator uses metaclass',
 'Not-nullable conn_type column in connection table',
 'Change signature of BigQueryGetDatasetTablesOperator',
 'Unify `hostname_callable` option in `core` section',
 'Changes in BigQueryHook',
 'Use project_id argument consistently across GCP hooks and operators',
 'GCSUploadSessionCompleteSensor signature change',
 'Remove SQL support in base_hook',
 'Changes to SalesforceHook',
 'Rename parameter name in PinotAdminHook.create_segment',
 'Rename parameter name in HiveMetastoreHook.get_partitions',
 'Remove unnecessary parameter in FTPHook.list_directory',
 'Remove unnecessary parameter in PostgresHook function copy_expert',
 'Change parameter name in OpsgenieAlertOperator',
 'Assigning task to a DAG using bitwise shift (bit-shift) operators are no longer supported',
 'Custom executors is loaded using full import path',
 'Removed sub-package imports from `airflow/__init__.py`',
 'Drop plugin support for stat_name_handler',
 'Standardize handling http exception in BigQuery',
 'Remove airflow.utils.file.TemporaryDirectory',
 'Chain and cross_downstream moved from helpers to BaseOperator',
 'Change python3 as Dataflow Hooks/Operators default interpreter',
 'Logging configuration has been moved to new section',
 'Simplification of CLI commands',
 'Remove gcp_service_account_keys option in airflow.cfg file',
 'Removal of airflow.AirflowMacroPlugin class',
 ' Change default aws_conn_id in EMR operators',
 'Removal of redirect_stdout, redirect_stderr',
 'Changes to SQLSensor',
 'Idempotency in BigQuery operators',
 'AWS Batch Operator',
 'Additional arguments passed to BaseOperator cause an exception',
 'Simplification of the TriggerDagRunOperator',
 'Changes in Google Cloud Platform related hooks',
 'Fernet is enabled by default',
 'Changes to Google PubSub Operators, Hook and Sensor',
 'Removed Hipchat integration',
 'The gcp_conn_id parameter in GKEPodOperator is required',
 'Changes to propagating Kubernetes worker annotations',
 'Remove provide_context',
 'Variables removed from the task instance context',
 'Moved provide_gcp_credential_file decorator to GoogleBaseHook',
 'Changes to S3Hook',
 'Changes to Google Transfer Operator',
 'Changes in  Google Cloud Transfer Hook',
 'CLI reorganization',
 'Removal of Mesos Executor',
 'Changes to SalesforceHook',
 'HTTPHook verify default value changed from False to True.',
 'Changes to GoogleCloudStorageHook',
 'Changes to CloudantHook',
 'Unify default conn_id for Google Cloud Platform',
 'Changes to sensor imports',
 'Deprecate legacy UI in favor of FAB RBAC UI',
 'CLI Changes',
 'Unification of `do_xcom_push` flag',
 'Changes to Dataproc related Operators',
 'Changes to skipping behaviour of LatestOnlyOperator',
 'Change default snowflake_conn_id for Snowflake hook and operators'

I like the idea proposed by @mik-laj in #9467 with defining Rules and that's something we should follow I think.

Questions that should be addressed before we move on:

If we are ok with the approach proposed by Kamil I can evaluate those changes and create issues so the work can be split.

turbaszek commented 4 years ago

@ash @mik-laj @potiuk @kaxil WDYT?

mik-laj commented 4 years ago

@turbaszek Can you create Google Docs about it? Without a broader context, the titles of changes are of little help.

I think we should now prepare a plan that will describe how we can detect breaking change and determine what to do when we detect them.

# Move operators to providers package (Rule 1)

We should load all DAG using DAGBag and check whether any deprecated operators are used. When it is detected, we can make changes automatically using Bowler.

This rule resolves the following entries from UPDATING.md
-....

A lot of changes are backward compatible, and if not, we also need to detect and notify them.

# Detectt provide_context in Python Operaator

We should check ....when this happens, we should inform the user that in Airflow this code may cause problems.

This rule resolves the following entries from UPDATING.md
- Remove provide_context
potiuk commented 4 years ago

I think some of the changes we can apply easily and safely. I think for example all the deprecation notices can be fixed rather easily - we already have information about it (we test all the depracations - I already used it in backport packages) - so we can super-easily and safely apply those in automated way. Similarly some of the changes above (rename parameter names etc.). I would really like to provide such tool to the users where it is no-brainer and can be automated.

For most other changes I think we should simply detect potential problems and flag them - providing helpful hint how the problem could be fixed.

And I agree GDOc about it might be better than issues/splitting them now. We can even split the work among ourselves and make recommendations individually for each of those - happy to share this with the rest. Then we can review them and turn into issues.

kaxil commented 4 years ago

We should definitely create the rules similar to what Kamil did in his PR and additionally have a flag that automatically fixes (where possible).

Google Doc, Confluence (https://cwiki.apache.org/confluence/display/AIRFLOW/) is fine if we want to review and add comments.

ashb commented 4 years ago

Gdoc fine for building up, but anything we point end users at should be in our official docs.

A mode to doc things automatically is desirable, but probably not on by default - as much as I wish everyone used git, I know some people out there don't, so we shouldn't change their files without confirmation (perhaps show a diff then ask y/n?)

turbaszek commented 4 years ago

so we shouldn't change their files without confirmation (perhaps show a diff then ask y/n?)

Bowler has this option

turbaszek commented 4 years ago

I've drafted this doc: https://docs.google.com/document/d/17tB9KZrH871q3AEafqR_i2I7Nrn-OT7le_P49G65VzM/edit?usp=sharing

The notes from UPDATING.md are split into few general groups (some are present in few places). I suppose that not all entries are important to users / will impact their Airflows.

mik-laj commented 4 years ago

@turbaszek I looked at this document. Great job!

I have looked at what changes we have and I think one tool may not solve all our problems. I think that apart from this command, we should take additional action.

I also added other ideas to new section - "Other ideas" in your docs.

PS. I created "area:upgrade" label on Github to track the related issues.

turbaszek commented 4 years ago
  • Complete the documentation to make the information for the user clearer, e.g. prepare a migration guide for CLI that will compare new and old commands

Yes I think the upgrade guide for CLI is a much better option than scrpits checking.

  • Many problems will only happen during the execution of the task. We can catch these exceptions and then save them to the database to make them more accessible to the user. The infinite log file is not the best way to communicate with the end-user. Especially when we do not have one fie, but an infinite number of them.

Interesting idea, but do you have any idea how to catch "these exceptions" (related to Airflow 2.0) not all?

mik-laj commented 4 years ago

Interesting idea, but do you have any idea how to catch "these exceptions" (related to Airflow 2.0) not all?

I think we can detect all problems and display them to the user. Why do you want to hide some problems?

turbaszek commented 4 years ago

So your idea is to collect all exceptions to db, not only those related to migration / Airflow2?

mik-laj commented 4 years ago

We can catch all the warnings, but those related to migration fall into one of two categories: DeprecationWarning, FutureWarning.

turbaszek commented 4 years ago

I got it, I was misled by the "exceptions" but we are talking about warnings

turbaszek commented 4 years ago

Base for airflow upgrade-check command is merged (#9276 ) so we can decide what rules we would like to implement. Once we have an initial list I will create issues so multiple people can work on this.

Based on this gdoc:

https://docs.google.com/document/d/17tB9KZrH871q3AEafqR_i2I7Nrn-OT7le_P49G65VzM/edit?usp=sharing

We may consider the following rules as a good start (those are major breaking changes):

and config related changes:

In case of operator related changes I would like to suggest a single rule OperatorChangesRule. It will use a map old_operator_name -> list of possible problems so we can create a single DagBag and scan all used operators and raise information about changes. It should also suggest what providers packages users should use. I think this is much better approach than having a rule for each operator...

I think we should first focus on the "check and warn" approach so we have any tool. Once we have it we may consider "check and apply changes" - even, in this case, we should probably first focus on major problems (config, new imports, etc). The second step can use "report" generated by airflow upgrade-check to apply changes (still can be single command but run wit additional flag).

Summoning all elders: @mik-laj @vikramkoka @ashb @potiuk @kaxil @feluelle @dimberman @houqp @ryw @Fokko

potiuk commented 4 years ago

Sounds great to start with. I am sure we will have more rules added by the users, but those seem like great to create issues for. And yeah. One rule to rule them all ("naming changes") sound great. But I'd name it "ImportChangesRule". We renamed Hooks, and Secret Backends and Sensors - not only operators :).

Elder Jarek.

kaxil commented 4 years ago

Yup I am happy with the suggested list