{"action":"created","issue":{"url":"https://api.github.com/repos/ga4gh/schemas/issues/253","labels_url":"https://api.github.com/repos/ga4gh/schemas/issues/253/labels{/name}","comments_url":"https://api.github.com/repos/ga4gh/schemas/issues/253/comments","events_url":"https://api.github.com/repos/ga4gh/schemas/issues/253/events","html_url":"https://github.com/ga4gh/schemas/pull/253","id":60510534,"number":253,"title":"Rationalises the SearchObjectRequest semantics.","user":{"login":"jeromekelleher","id":2664569,"avatar_url":"https://avatars.githubusercontent.com/u/2664569?v=3","gravatar_id":"","url":"https://api.github.com/users/jeromekelleher","html_url":"https://github.com/jeromekelleher","followers_url":"https://api.github.com/users/jeromekelleher/followers","following_url":"https://api.github.com/users/jeromekelleher/following{/other_user}","gists_url":"https://api.github.com/users/jeromekelleher/gists{/gist_id}","starred_url":"https://api.github.com/users/jeromekelleher/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jeromekelleher/subscriptions","organizations_url":"https://api.github.com/users/jeromekelleher/orgs","repos_url":"https://api.github.com/users/jeromekelleher/repos","events_url":"https://api.github.com/users/jeromekelleher/events{/privacy}","received_events_url":"https://api.github.com/users/jeromekelleher/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":13,"created_at":"2015-03-10T14:45:50Z","updated_at":"2015-03-29T16:48:52Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/ga4gh/schemas/pulls/253","html_url":"https://github.com/ga4gh/schemas/pull/253","diff_url":"https://github.com/ga4gh/schemas/pull/253.diff","patch_url":"https://github.com/ga4gh/schemas/pull/253.patch"},"body":"This PR changes the semantics of all `SearchObjectRequest`s so they require the ID of a single container object, if applicable. Currently, a number of different conventions exist, making implementation and authorisation difficult. This solves issue #247 and is related to the issues in #248.\r\n\r\nMost of the changes are straightforward, and just require the change of a field like (for example) `array<string> dataSetIds = [];` to `string dataSetId;`. There are some problematic cases:\r\n\r\n1. `SearchCallsRequest` and `SearchAlleleCallsRequest` both had a large list of options for the containers that they could search over. As an initial strawman I've reduced this to just the parent `CallSet` to be consistent with all the other objects. Reasoning for these:\r\n   1. Since a `CallSet` is part of a `VariantSet`, the variant set is implied by specifying `callSetId`.\r\n   2. Finding  `Calls` for a particular `variantId` is redundant, since `Call`s are embedded in `Variant` objects. If we wish to find the calls for a given variant, we can either find them directly using `SearchVariantsRequest` or make a call to GET `/variants/<id>/`\r\n   3. We are then left with iterating over all calls in a CallSet, which seems like a peculiar thing to do without reference to the containing `Variant` object. However, this might be updated with more meaningful semantics later.\r\n2. `SeachAlleles` is problematic; what is the interplay between the `variantSetId` and the `sequenceId`? If `sequenceId` is required, then why do we need the `variantSetId`? Conceptually, it seems natural that an Allele belongs to a single `VariantSet`.\r\n\r\nIn terms of backwards compatibility, obviously there are some breaking changes here. These are minor, however, and easily worked around. Server-side, this is a considerable simplification, and so implementation can simply be a special case of the existing semantics. Client-side, users that are using the multi-container search semantics will need to now explicitly iterate over the container IDs that they are interested in."},"comment":{"url":"https://api.github.com/repos/ga4gh/schemas/issues/comments/87442162","html_url":"https://github.com/ga4gh/schemas/pull/253#issuecomment-87442162","issue_url":"https://api.github.com/repos/ga4gh/schemas/issues/253","id":87442162,"user":{"login":"dglazer","id":5824733,"avatar_url":"https://avatars.githubusercontent.com/u/5824733?v=3","gravatar_id":"","url":"https://api.github.com/users/dglazer","html_url":"https://github.com/dglazer","followers_url":"https://api.github.com/users/dglazer/followers","following_url":"https://api.github.com/users/dglazer/following{/other_user}","gists_url":"https://api.github.com/users/dglazer/gists{/gist_id}","starred_url":"https://api.github.com/users/dglazer/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dglazer/subscriptions","organizations_url":"https://api.github.com/users/dglazer/orgs","repos_url":"https://api.github.com/users/dglazer/repos","events_url":"https://api.github.com/users/dglazer/events{/privacy}","received_events_url":"https://api.github.com/users/dglazer/received_events","type":"User","site_admin":false},"created_at":"2015-03-29T16:48:52Z","updated_at":"2015-03-29T16:48:52Z","body":"@jeromekelleher, that multi-BAM change addresses the biggest concern I saw raised -- thanks.\r\n\r\nThere are 12 separate changes in this PR, and I was finding the diff's hard to read, so I made the summary below. Assuming I got it right, I:\r\n- am +0 on the 6 \"not yet published\" changes (they look reasonable, but I haven't been part of those design discussions)\r\n- am +1 on the 2 \"published but not implemented\" changes\r\n- would like to be sure there aren't any bad consequences to the 4 \"already implemented\" changes.\r\n\r\nA few questions:\r\n- @jeromekelleher -- does it look like I correctly caught all twelve changes?\r\n- @calbach -- does it look like I binned them correctly into \"not yet published\", \"published but not implemented\", and \"already implemented\"? (I think I got them right, but please double-check.)\r\n- @calbach -- of the 4 potentially breaking changes, do you have any thoughts on which if any could cause real problems in existing code?\r\n\r\n#### 6 not yet published APIs\r\n*Backwards compatibility isn't an issue here.*\r\n\r\n**src/main/resources/avro/metadatamethods.avdl**\r\n```\r\nrecord SearchIndividualsRequest {\r\n-  array<string> groupIds = [];\r\n+  string groupId;\r\n\r\nrecord SearchSamplesRequest {\r\n-  array<string> individualIds = [];\r\n+  string individualId;\r\n```\r\n**src/main/resources/avro/variantmethods.avdl**\r\n```\r\nrecord SearchVariantSetSequencesRequest {\r\n-  array<string> parentIds = [];\r\n+  string parentId;\r\n\r\nrecord SearchAllelesRequest {\r\n-  array<string> variantSetIds = [];\r\n+  string variantSetId;\r\n\r\nrecord SearchCallsRequest { \r\n-  array<string> callSetIds = [];\r\n-  array<string> variantSetIds = [];\r\n-  array<string> variantIds = [];\r\n+  string callSetId;\r\n\r\nrecord SearchAlleleCallsRequest { \r\n-  array<string> callSetIds = [];\r\n-  array<string> variantSetIds = [];\r\n-  array<string> alleleIds = [];\r\n-  array<string> variantIds = [];\r\n+  string callSetId;\r\n```\r\n\r\n#### 2 published APIs, with unimplemented behavior\r\n*As far as I know these changes don't break any existing implementations, so again backwards compatibility isn't an issue.*\r\n**src/main/resources/avro/variantmethods.avdl**\r\n```\r\nrecord SearchVariantSetsRequest {\r\n-  array<string> datasetIds = [];\r\n+  string datasetId;\r\n\r\nrecord SearchVariantsRequest {\r\n-  array<string> variantSetIds = [];\r\n+  string variantSetId;\r\n```\r\n\r\n#### 4 published APIs, with implemented behavior\r\n*These changes are small, but might cause a backwards compatibility issue.*\r\n**src/main/resources/avro/variantmethods.avdl**\r\n```\r\nrecord SearchCallSetsRequest {\r\n-  array<string> variantSetIds = [];\r\n+  string variantSetId;\r\n```\r\n**src/main/resources/avro/readmethods.avdl**\r\n```\r\nrecord SearchReadsRequest {\r\n-  array<string> readGroupIds = [];\r\n+  array<string> readGroupIds;\r\n\r\nrecord SearchReadGroupSetsRequest {\r\n-  array<string> datasetIds = [];\r\n+  string datasetId;\r\n```\r\n**src/main/resources/avro/referencemethods.avdl**\r\n```\r\nrecord SearchReferencesRequest {\r\n-  union { null, string } referenceSetId = null;\r\n+  string referenceSetId;\r\n```\r\n"}}