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

Added new datetime format to support windows FTP Server #181

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

fti-akumari
Copy link

@fti-akumari fti-akumari commented Dec 19, 2024

Description:-

This PR enhances the parse_list_line_windows method to support multiple date/time formats when parsing Microsoft Windows dir command output. Previously, only one specific date/time format was supported. The following improvements were made:

Changes:

  • Added support for two date/time formats:
    "%m/%d/%Y %I:%M %p" (e.g., 12/18/2024 04:40 PM)
    "%m-%d-%y %I:%M%p" (e.g., 12-18-24 04:40PM)

  • Introduced a loop to try each format and parse the date/time correctly.

  • Raised a ValueError if none of the formats match, ensuring robust error handling.

Why:
Windows FTP servers may use different date/time formats depending on system configurations and locale settings. Supporting multiple formats improves compatibility and reduces parsing failures.

image (9)

strptime = datetime.datetime.strptime
date_time = strptime(date_time_str, "%m/%d/%Y %I:%M %p")
for fmt in possible_formats:
try:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use contextlib.suppress if you don't need to handle exception, but want to ignore it

break
except ValueError:
continue
if not date_time:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use "else" branch for "for" loop instead of value inspection

@@ -501,9 +501,19 @@ def parse_list_line_windows(self, b):
date_time_str = line[: date_time_end + 1].strip().split(" ")
date_time_str = " ".join([x for x in date_time_str if len(x) > 0])
line = line[date_time_end + 1 :].lstrip()

possible_formats = ["%m/%d/%Y %I:%M %p", "%m-%d-%y %I:%M%p"]
Copy link
Collaborator

@pohmelie pohmelie Dec 20, 2024

Choose a reason for hiding this comment

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

I think this should be multiline, so new rules can be added without commit history loss

Copy link

codecov bot commented Dec 20, 2024

Codecov Report

Attention: Patch coverage is 70.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 98.68%. Comparing base (8c774d8) to head (48ed29d).

Files with missing lines Patch % Lines
src/aioftp/client.py 70.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #181      +/-   ##
==========================================
- Coverage   98.83%   98.68%   -0.16%     
==========================================
  Files           6        6              
  Lines        1896     1904       +8     
==========================================
+ Hits         1874     1879       +5     
- Misses         22       25       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants