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

Return (entity-)type for collections #170

Merged
merged 3 commits into from
Jul 21, 2015
Merged

Return (entity-)type for collections #170

merged 3 commits into from
Jul 21, 2015

Conversation

JonnyJD
Copy link
Collaborator

@JonnyJD JonnyJD commented Jun 10, 2015

This is part of #154.

This works fine so far, but lists of events can no yet be returned because events are not yet implemented (see #168).

Getting the type of a collection by MBID is a bit clumsy.
If the collection is not from the user that is currently authenticated we basically guess that it is of type release and request the list of releases. We will request events otherwise later.

Maybe there is a better option to find out the type, see:
http://chatlogs.musicbrainz.org/musicbrainz-devel/2015/2015-06/2015-06-10.html#T16-17-51-539040

@alastair
Copy link
Owner

👍

@JonnyJD JonnyJD added this to the 0.6 milestone Jun 10, 2015
@JonnyJD JonnyJD self-assigned this Jun 10, 2015
@JonnyJD
Copy link
Collaborator Author

JonnyJD commented Jun 10, 2015

Hm, finding the type still needs more work in the examle. Not part of the API, but having a working example is crucial for a) documentation b) checking of the API is "sane":

When the collection I request is public, then I can't use get_collections() to find out the type, since that itself always needs authentication. Authentication is tricky with collections. (see #155)

Additionally it might be wise to wait for #168 to complete so we can add full event collection support in this PR.

Signed-off-by: Johannes Dewender <[email protected]>
The example itself still runs on mbngs 0.5 and doesn't display
the types in this case.
A request for a collection of events fails with a message
that events are not yet implemented.

Signed-off-by: Johannes Dewender <[email protected]>
Signed-off-by: Johannes Dewender <[email protected]>
@JonnyJD
Copy link
Collaborator Author

JonnyJD commented Jun 15, 2015

I updated the example to reduce the complexity for checking the type (just tries /release and catchs a ResponseError. That is the best we can do in general. Depending on the application the user might actually know what type of collection it is, but I didn't want to clutter the command line interface for the example.

I also added XML tests for collections.

I will update the branch again when events are merged (and implement events for collections).

@alastair
Copy link
Owner

I don't think we should wait for #168 for this. Note that a work read endpoint has been released: http://tickets.musicbrainz.org/browse/MBS-8459 but we still need write. I wonder if we can rework the collection methods to make this easier to use.

@alastair
Copy link
Owner

I'm happy to 🚢 as-is

@JonnyJD JonnyJD merged commit 3ff2bfe into alastair:master Jul 21, 2015
JonnyJD added a commit that referenced this pull request Jul 21, 2015
see pull request #170

Signed-off-by: Johannes Dewender <[email protected]>
@JonnyJD JonnyJD deleted the collection_type branch July 21, 2015 20:32
@JonnyJD
Copy link
Collaborator Author

JonnyJD commented Jul 21, 2015

Merged and added #175 for getting lists of events (blocked by #168).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants