[COST-5627] Fix Azure default extension issue #5394
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5394 +/- ##
=====================================
Coverage 94.1% 94.1%
=====================================
Files 371 371
Lines 31540 31535 -5
Branches 3379 3378 -1
=====================================
- Hits 29689 29686 -3
+ Misses 1200 1199 -1
+ Partials 651 650 -1 |
bacciotti
added
the
azure-smoke-tests
|
/retest |
|
/retest |
There was a problem hiding this comment.
How did you test with a manifest? Because I'm fairly sure even if we have one it will just run as if we don't. The code here probably works but its also confusing in places (the azure downloader in general at this point) because of how we treat manifests. We may want to clean up all the extension logic and correctly download all manifest types to reduce the complexity of code overall. Happy to chat through all this if its not clear.
|
/retest |
| if extension: | ||
| if not blob.name.endswith(extension): | ||
| continue | ||
| elif not any(blob.name.endswith(ext) for ext in valid_extensions): |
There was a problem hiding this comment.
Should we just drop this logic and the valid extensions list above? Essentially if we are not trying to download a manifest this elif statement is going to mangle file types together anyway so its kinda redundant no?
There was a problem hiding this comment.
Dropping this logic broke the tests, which indicates that this filtering is needed. Dropping this logic, we can lose control over the extensions used, I think. Maybe we can keep it to maintain the consistency and align with the existing tests...
|
/retest |
1 similar comment
|
/retest |
|
/retest |
2 similar comments
|
/retest |
|
/retest |
lcouzens
added
smoke-tests
|
/retest |
2 similar comments
|
/retest |
|
/retest |
bacciotti
deleted the
cost-5627-azure-does-not-change-default-extension
branch
Jira Ticket
COST-5627
Description
This change fixes the issue of a hardcoded file extension (csv) when no manifest is present. Now, the program dynamically identifies and selects the latest file with supported extensions, such as
.csvor.gz.Testing
csvandcsv.gzdata with and without amainfest.jsonfiletox -e py311 -- masu.test.external.downloader.azureRelease Notes