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

fix ogg audio #2085

Merged
merged 10 commits into from
Nov 16, 2023
Merged

fix ogg audio #2085

merged 10 commits into from
Nov 16, 2023

Conversation

AndreaFrancis
Copy link
Contributor

Fix for #2045

@codecov-commenter
Copy link

codecov-commenter commented Nov 9, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (4238a17) 90.17% compared to head (65b3653) 87.45%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2085      +/-   ##
==========================================
- Coverage   90.17%   87.45%   -2.73%     
==========================================
  Files         262      118     -144     
  Lines       15945     4447   -11498     
==========================================
- Hits        14379     3889   -10490     
+ Misses       1566      558    -1008     
Flag Coverage Δ
jobs_cache_maintenance 95.33% <ø> (ø)
jobs_mongodb_migration 86.69% <ø> (ø)
libs_libapi ?
libs_libcommon ?
services_admin 86.56% <ø> (ø)
services_api 87.09% <ø> (ø)
services_rows 85.49% <ø> (ø)
services_search 80.48% <ø> (ø)
services_sse-api 94.21% <ø> (ø)
services_worker ?

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@AndreaFrancis AndreaFrancis requested a review from lhoestq November 9, 2023 20:58
@AndreaFrancis AndreaFrancis marked this pull request as ready for review November 9, 2023 21:01
@AndreaFrancis AndreaFrancis requested a review from severo November 10, 2023 17:05
@severo
Copy link
Collaborator

severo commented Nov 10, 2023

maybe add a test to ensure that ogg files were not working before, and are working now?

@AndreaFrancis AndreaFrancis requested a review from a team November 15, 2023 17:48
@AndreaFrancis
Copy link
Contributor Author

maybe add a test to ensure that ogg files were not working before, and are working now?

I added a specific test. It was needed to mock s3 bucket since the issue only appears when using fsspec+s3fs.
Note that I had to do a couple of patches because of aiobotocore getmoto/moto#6836 and aio-libs/aiobotocore#755
MockRawResponse' object has no attribute 'raw_headers' error when using moto with s3fs

@severo
Copy link
Collaborator

severo commented Nov 15, 2023

wow, it was more complex than expected. Maybe another review before merging?

@AndreaFrancis
Copy link
Contributor Author

wow, it was more complex than expected. Maybe another review before merging?

Sure.
I will fix the import error in libcommon unittests:

ImportError while importing test module '/home/runner/work/datasets-server/datasets-server/libs/libcommon/tests/viewer_utils/test_features.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
tests/viewer_utils/test_features.py:12: in <module>
    import boto3
E   ModuleNotFoundError: No module named 'boto3'

@AndreaFrancis AndreaFrancis merged commit 62f589b into main Nov 16, 2023
22 checks passed
@AndreaFrancis AndreaFrancis deleted the fix-ogg-conversion branch November 16, 2023 12:23
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.

3 participants