tamland / python-tidal

Python API for TIDAL music streaming service
GNU Lesser General Public License v3.0
402 stars 109 forks source link

Add support for get track by ISRC and get album by BarcodeID #274

Closed M4TH1EU closed 3 weeks ago

M4TH1EU commented 3 weeks ago

I had to add support for a custom base url in request() and basic_request() to use the new api.

See here for API documentation : https://apiref.developer.tidal.com Uses the filter[x] param.

Shouldn't have any impact on the other functionalities.

Would fix https://github.com/tamland/python-tidal/issues/96

tehkillerbee commented 3 weeks ago

Thanks for this PR, much appreciated. Yourr additions generally looks good to me. Only comment would be to specify the openapi v2 path in the session config and not pass it directly.

While you could probably have avoided to modify the basic_request method (as done in user.favourites.mixes where the v2 api url is used):

  def mixes(self, limit: Optional[int] = 50, offset: int = 0) -> List["MixV2"]:
      """Get the users favorite mixes & radio.

      :return: A :class:`list` of :class:`~tidalapi.media.Mix` objects containing the user favourite mixes & radio
      """
      params = {"limit": limit, "offset": offset}
      return cast(
          List["MixV2"],
          self.requests.map_request(
              url=urljoin("https://api.tidal.com/v2/", f"{self.v2_base_url}/mixes"),
              params=params,
              parse=self.session.parse_v2_mix,
          ),
      )

I like your approach better so I will merge this and modify the mixes function accordingly.

I'd also prefer to use the exception ObjectNotFound instead of an HTTPError to allow python-tidal to fail gracefully.

Both functions work as expected when using isrc: "USSM12209515" and barcode "196589525444" respectively.

M4TH1EU commented 3 weeks ago

I looked for a method with a manually specified base url but couldn't find one such as that mixes() one. Lots of lines 😅

I though about using ObjectNotFound but if I recall correctly the request threw HTTPError instead so that is why i used that one. That would need verifying though.

tehkillerbee commented 3 weeks ago

I looked for a method with a manually specified base url but couldn't find one such as that mixes() one. Lots of lines 😅

I though about using ObjectNotFound but if I recall correctly the request threw HTTPError instead so that is why i used that one. That would need verifying though.

Yes, I believe you are correct. I will add a separate check to validate if the returned tracks/albums are empty and perhaps throw an exception in this case :)

tehkillerbee commented 3 weeks ago

@M4TH1EU Looks like there are some linter errors, that must be fixed before merging this PR.

EDIT: On second thought I will fix the linter errors in a separate PR (based on this one), as I have made some additional changes that should be merged in at the same time.