Skip to content

Conversation

@mik-laj
Copy link
Member

@mik-laj mik-laj commented Feb 2, 2020

Some people love the latest syntax, so I would like to introduce it automatically for the project. These are mainly fstring changes, but not only.


Issue link: AIRFLOW-6719

Make sure to mark the boxes below before creating PR: [x]

  • Description above provides context of the change
  • Commit message/PR title starts with [AIRFLOW-NNNN]. AIRFLOW-NNNN = JIRA ID*
  • Unit tests coverage for changes (not needed for documentation changes)
  • Commits follow "How to write a good git commit message"
  • Relevant documentation is updated including usage instructions.
  • I will engage committers as explained in Contribution Workflow Example.

* For document-only changes commit message can start with [AIRFLOW-XXXX].


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.
Read the Pull Request Guidelines for more information.

@boring-cyborg boring-cyborg bot added area:CLI area:dev-tools area:Scheduler including HA (high availability) scheduler k8s labels Feb 2, 2020
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.

Love it !

@mik-laj
Copy link
Member Author

mik-laj commented Feb 2, 2020

The first build is unlikely to be valid because pyupgrade introduces changes incompatible with flake8, but I want to check it on CI.

@mik-laj mik-laj force-pushed the AIRFLOW-6719-pyupgrade branch 2 times, most recently from b18ba32 to 918a3e5 Compare February 18, 2020 01:07
@mik-laj mik-laj force-pushed the AIRFLOW-6719-pyupgrade branch from 918a3e5 to 33c69ab Compare February 18, 2020 01:10
Copy link
Member

@zhongjiajie zhongjiajie left a comment

Choose a reason for hiding this comment

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

I think is ok, for now I find out Travis sad due to some nit

Comment on lines +77 to +89
<<<<<<< HEAD
s3_hook.delete_objects(bucket=self.bucket, keys=self.keys)
=======

response = s3_hook.delete_objects(bucket=self.bucket, keys=self.keys)

deleted_keys = [x['Key'] for x in response.get("Deleted", [])]
self.log.info("Deleted: %s", deleted_keys)

if "Errors" in response:
errors_keys = [x['Key'] for x in response.get("Errors", [])]
raise AirflowException(f"Errors when deleting: {errors_keys}")
>>>>>>> b18ba328b... [AIRFLOW-6719] Introduce pyupgrade to enforce latest syntax
Copy link
Member

Choose a reason for hiding this comment

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

It fail as you said, not that easy, but this is rebase mistake

ssh_hook=self.hook,
command="echo '{0}' > {1}".format(test_remote_file_content,
command="echo '{}' > {}".format(test_remote_file_content,
self.sftp_path),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
self.sftp_path),
self.sftp_path),
tests/providers/amazon/aws/operators/test_sftp_to_s3.py:85:47: E127 continuation line over-indented for visual indent
tests/models/test_taskinstance.py:318:45: E128 continuation line under-indented for visual indent
airflow/ti_deps/deps/runnable_exec_date_dep.py:38:47: E127 continuation line over-indented for visual indent
tests/providers/sftp/operators/test_sftp.py:214:47: E127 continuation line over-indented for visual indent
tests/providers/sftp/operators/test_sftp.py:252:47: E127 continuation line over-indented for visual indent
tests/providers/sftp/operators/test_sftp.py:291:47: E127 continuation line over-indented for visual indent
tests/providers/sftp/operators/test_sftp.py:327:47: E127 continuation line over-indented for visual indent
airflow/cli/commands/dag_command.py:85:50: E127 continuation line over-indented for visual indent
airflow/ti_deps/deps/dag_ti_slots_available_dep.py:33:57: E127 continuation line over-indented for visual indent
airflow/providers/sftp/operators/sftp.py:139:57: E127 continuation line over-indented for visual indent
airflow/providers/sftp/operators/sftp.py:150:57: E127 continuation line over-indented for visual indent
airflow/ti_deps/deps/not_in_retry_period_dep.py:52:41: E127 continuation line over-indented for visual indent

@mik-laj
Copy link
Member Author

mik-laj commented Feb 27, 2020

@zhongjiajie I don't have time to finish this, because I've taken care of the scheduler performance. Would you like to take over?

@zhongjiajie
Copy link
Member

@mik-laj Sure, but how could I do? submit a new PR to this repo? or create a PR to PolideaInternal:AIRFLOW-6719-pyupgrade?

@mik-laj
Copy link
Member Author

mik-laj commented Feb 27, 2020

@mik-laj please create new PR to apache/airflow but reuse JIRA number.

@ashb
Copy link
Member

ashb commented Feb 28, 2020

Closed in favour of #7573

@ashb ashb closed this Feb 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:CLI area:dev-tools area:Scheduler including HA (high availability) scheduler

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants