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

Refactor os.path.splitext to Path.* #34352

Merged
merged 1 commit into from
Sep 15, 2023
Merged

Refactor os.path.splitext to Path.* #34352

merged 1 commit into from
Sep 15, 2023

Conversation

eumiro
Copy link
Contributor

@eumiro eumiro commented Sep 13, 2023

With Python 3.9+ we can change that:

pidlock_path = pid_path.with_name(f"{pid_path.stem}-monitor{pid_path.suffix}")

to

pidlock_path = pid_path.with_stem(f"{pid_path.stem}-monitor")

@boring-cyborg boring-cyborg bot added area:CLI area:Scheduler including HA (high availability) scheduler area:webserver Webserver related Issues kind:documentation labels Sep 13, 2023
Copy link
Member

@hussein-awala hussein-awala left a comment

Choose a reason for hiding this comment

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

Looks good

@@ -1042,8 +1040,7 @@ def _collect_results_from_processor(self, processor) -> None:
run_count=self.get_run_count(processor.file_path) + 1,
)
self._file_stats[processor.file_path] = stat

file_name = os.path.splitext(os.path.basename(processor.file_path))[0].replace(os.sep, ".")
Copy link
Member

Choose a reason for hiding this comment

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

The final replace call is so weird… Since basename is called, the value just can never contain an os.sep in the first place.

@uranusjr uranusjr merged commit 4869575 into apache:main Sep 15, 2023
43 checks passed
@eumiro eumiro deleted the splitext branch September 15, 2023 14:49
@ephraimbuddy ephraimbuddy added this to the Airflow 2.7.2 milestone Oct 3, 2023
@ephraimbuddy ephraimbuddy added the type:misc/internal Changelog: Misc changes that should appear in change log label Oct 3, 2023
ephraimbuddy pushed a commit that referenced this pull request Oct 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:CLI area:Scheduler including HA (high availability) scheduler area:webserver Webserver related Issues kind:documentation type:misc/internal Changelog: Misc changes that should appear in change log
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants