-
Notifications
You must be signed in to change notification settings - Fork 4
feat: add csv_parser to parse legacy OTA image's metadata CSV files into sqlite3 db #466
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
Conversation
for more information, see https://pre-commit.ci
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, left one comments. Feel better to use csv library.
for _idx, line in enumerate(f, start=1): | ||
_new = parse_dirs_csv_line(line) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this parser is only for csv, how about to use the default CSV library instead of regex? I imagine the reason to use regex is to re-use the existing parser for text, but feel that csv library for csv parser is more safe.
Another idea is to aggregate regext module as helper function between text and csv. It might be preference, but I don't mind to create/use such utility functions between Classes.
https://docs.python.org/3/library/csv.html
@airkei 😢 I do like to, but again, the CSV format of the legacy OTA image is kind of nonstandard, long-long time ago I did want to use csv lib, but then I gave up. |
( Since ancient time of this repository, the regex was in used to parse the CSV. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, in that case, I agree with proceeding this approach.
I also feel it might be much more maintainable to isolate legacy modules completely rather than creating utility function...
Could you elaborate your idea with more details? |
Sure! currently there is code clone between
My idea is to create a single function and aggregate these regex parts between parser and csv_parser. |
No worry, this PR is part of my effort to rewrite the |
Introduction
Note
This PR is split from #453 . The PR only introduces the implementation of csv_parser, but not yet integrate it into otaclient code, also the previous legacy OTA image parser implementation is not changed in this PR.
This PR introduces the implementation of parsing legacy OTA image metadata CSV files into file_table entries, and importing the parsed results as sqlite3 tables.
NOTE that the introduced implementation is not yet integrated into otaclient code, will be integrated in other future PR.