Fixing minor item on csv parser #2
Conversation
WalkthroughThe changes primarily focus on enhancing the airport data parsing process in the Go application. A CSV parser is introduced to parse the fetched data, and the error handling logic is improved to skip record-level errors, ensuring maximum data processing. Changes
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Files ignored due to filter (4)
- functions/src/init/go.mod
- functions/src/init/go.sum
- go.mod
- go.sum
Files selected for processing (2)
- functions/src/init/main.go (2 hunks)
- pkg/airport/parsers/csv/csv.go (1 hunks)
Files skipped from review due to trivial changes (1)
- pkg/airport/parsers/csv/csv.go
Additional comments (Suppressed): 5
functions/src/init/main.go (5)
4-7: The new import statements are for the "bytes" package and a custom CSV parser package. Ensure that the "github.com/tarmac-project/example-airport-lookup-go/pkg/airport/parsers/csv" package is available and accessible.
[APPROVE]18-22: The fetched airport data is now being stored in the
datavariable. This is a necessary change for the new CSV parser.
[APPROVE]25-29: The code creates a new CSV parser with the fetched data. If there's an error during this process, it's logged and returned. This is a good practice for error handling.
[APPROVE]31-35: The code parses the airport data using the CSV parser. If there's an error during parsing, it's logged and returned. This is a good practice for error handling.
[APPROVE]36-36: The number of fetched airports is logged. This could be useful for debugging and monitoring.
[APPROVE]
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2 +/- ##
==========================================
+ Coverage 92.07% 92.92% +0.85%
==========================================
Files 2 2
Lines 101 99 -2
==========================================
- Hits 93 92 -1
+ Misses 6 5 -1
Partials 2 2
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 4
Files selected for processing (2)
- functions/src/init/main.go (2 hunks)
- pkg/airport/parsers/csv/csv.go (1 hunks)
Additional comments (Suppressed): 1
functions/src/init/main.go (1)
- 4-6: The new import statements for "bytes" and the custom CSV parser package are appropriate for the changes made in this PR. Ensure that the "github.com/tarmac-project/example-airport-lookup-go/pkg/airport/parsers/csv" package is available and accessible.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Files selected for processing (1)
- functions/src/init/main.go (2 hunks)
Additional comments (Suppressed): 3
functions/src/init/main.go (3)
4-6: The new import statements for the
bytespackage and the custom CSV parser package are appropriate for the changes made in this PR. Thebytespackage is used to create a new reader for the fetched data, and the CSV parser package is used to parse the airport data.18-22: The error message in the log has been updated to include the error returned from the
fetchfunction call. This is a good practice as it provides more context in the logs when an error occurs.25-35: The new code creates a CSV parser with the fetched data, parses the airport data, and logs the number of fetched airports. It also includes error handling for the creation and parsing process. This is a significant improvement over the previous code, which did not parse the data or log the number of fetched airports.
However, there is a potential issue with the error handling. If an error occurs during the creation of the CSV parser or during the parsing process, the function returns immediately. This means that if there is an error with one record, the entire parsing process will be aborted. This could be problematic if the data contains many records and only a few of them have errors. It might be better to handle these errors in a way that allows the function to continue processing the remaining records.
airports, err := parser.Parse() if err != nil { - f.tarmac.Logger.Error(fmt.Sprintf("Failed to parse airport data - %s", err)) - return []byte(""), fmt.Errorf("Failed to parse airport data: %s", err) + f.tarmac.Logger.Warn(fmt.Sprintf("Failed to parse some airport data - %s", err)) }
Summary by CodeRabbit