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

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

Closed

Conversation

M4TH1EU
Copy link
Contributor

@M4TH1EU M4TH1EU commented Sep 4, 2024

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 #96

@tehkillerbee
Copy link
Collaborator

tehkillerbee commented Sep 5, 2024

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
Copy link
Contributor Author

M4TH1EU commented Sep 5, 2024

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
Copy link
Collaborator

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
Copy link
Collaborator

tehkillerbee commented Sep 5, 2024

@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.

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.

Feature suggestion: add tracks to favorites or to playlist using list of ISRC's
2 participants