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 mediadb str issues #44

Merged
merged 32 commits into from
Mar 11, 2023
Merged

Fix mediadb str issues #44

merged 32 commits into from
Mar 11, 2023

Conversation

palfrey
Copy link
Contributor

@palfrey palfrey commented Feb 6, 2022

There's an issue with the use of str in mediadb, and a lack of tests for it meant it didn't turn up, so I've fixed that.

Needs #58 merging first for build fixes

@palfrey palfrey marked this pull request as ready for review February 12, 2022 15:26
@codecov
Copy link

codecov bot commented Feb 19, 2022

Codecov Report

Merging #44 (51db456) into master (25e6a85) will decrease coverage by 0.25%.
The diff coverage is 63.63%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #44      +/-   ##
==========================================
- Coverage   47.27%   47.02%   -0.25%     
==========================================
  Files          50       52       +2     
  Lines       10826    11408     +582     
==========================================
+ Hits         5118     5365     +247     
- Misses       5708     6043     +335     
Flag Coverage Δ
unittest 47.02% <63.63%> (-0.25%) ⬇️

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

Impacted Files Coverage Δ
coherence/backends/fs_storage.py 43.26% <0.00%> (ø)
coherence/upnp/core/service.py 66.44% <0.00%> (ø)
coherence/upnp/devices/media_server.py 33.33% <ø> (ø)
coherence/base.py 58.23% <40.00%> (-0.44%) ⬇️
coherence/upnp/core/device.py 73.41% <66.66%> (-0.06%) ⬇️
coherence/backends/mediadb_storage.py 54.22% <100.00%> (ø)
coherence/upnp/devices/control_point.py 38.33% <100.00%> (ø)

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d39afd4...51db456. Read the comment docs.

@palfrey palfrey marked this pull request as draft February 19, 2022 16:01
@palfrey palfrey marked this pull request as ready for review February 20, 2022 10:41
@palfrey
Copy link
Contributor Author

palfrey commented Jan 8, 2023

@opacam Any thoughts? I've just hit these again

Copy link
Owner

@opacam opacam left a comment

Choose a reason for hiding this comment

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

LGTM!! 😄 , So many thanks and my apologies for not reviewing this before...

Note: I will leave this approved without merging for a few days, just in case that you want to address the comments. Otherwise don't worry, I'll do it after the merging. Thanks again!!!

.github/workflows/push.yml Outdated Show resolved Hide resolved
Comment on lines +588 to +591
album: bytes = tags.get('album', b'')
artist: bytes = tags.get('artist', b'')
title: bytes = tags.get('title', b'')
track: int = tags.get('track', 0)
Copy link
Owner

Choose a reason for hiding this comment

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

Nice, some Python types!!! 😄

pyproject.toml Outdated
twisted = ">=20.3.0, <22.1.0" # 22.1.0 removed HTTPPageGetter as it's now deprecated
"zope.interface" = "*"

# dev-dependencies
flake8 = {optional = true, version = "*"}
flake8 = {optional = true, version = ">=4"} # Because of https://github.com/PyCQA/flake8/issues/1564
Copy link
Owner

Choose a reason for hiding this comment

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

👍

tests/backends/test_mediadb_storage.py Outdated Show resolved Hide resolved
@opacam opacam merged commit e889577 into opacam:master Mar 11, 2023
@opacam
Copy link
Owner

opacam commented Mar 11, 2023

🚀 Merged
Thanks again @palfrey !!!! 😄

@palfrey palfrey deleted the mediadb branch March 11, 2023 18:57
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