IQSS / dataverse

Open source research data repository software
http://dataverse.org
Other
878 stars 490 forks source link

Breaking API changes in 4.12: RSpace (and other?) integrations broken #5724

Closed richarda23 closed 5 years ago

richarda23 commented 5 years ago

Hi Since 4th April our acceptance tests for the Java Dataverse SDK are failing running against demo dataverse, I expect this is due to updating to 4.12 . I'm wondering if there are any known changes to the returned data structure from a call to https://demo.dataverse.org/api/v1/dataverses/<dataverseAlias>? From the error messages we are getting it may be something to do with cardinality of dataverseContacts field.

Just more broadly, is there a published changelog of API changes when a new version of Dataverse is released? Is it possible to access a demo version of the previous version (4.11 in this case) for a short time after a new release for comparison?

Thanks Richard

pdurbin commented 5 years ago

@richarda23 hi! I'm sorry to hear we may have broken something. I guess is that pull request #5655 could have changed something you're relying on. I haven't looked deeply at it. If you spot something, please let us know.

When you say Java SDK I assume you mean https://github.com/IQSS/dataverse-client-java . It would be great to have some public continuous integration set up for it. It looks like we talked about this a bit at https://github.com/IQSS/dataverse-client-java/issues/5 . (I have some news I'll share on that issue.)

We don't publish any changelogs (we probably should, especially if you ask the https://keepachangelog.com people :smile: ). We do tag all issues and pull requests with GitHub milestones, if that helps. It's sometimes a lot to wade through.

IQSS generally only has the latest release in production and on our demo site. For public APIs like this I will admit that sometimes I poke around installations that are out there in the community. You can get a list from the bottom of https://dataverse.org/metrics

I hope this helps. Thanks for the bug report!

poikilotherm commented 5 years ago

It should be fairly easy to get a v4.11 instance running with IQSS/dataverse-kubernetes.

richarda23 commented 5 years ago

Hi Phil, thanks for quick reply

We have a lot of integrations with other software now and aren't really able to follow every single issue for all projects on the offchance that a change might affect the API. An API changelog can be really simple/low effort, we just use a public GoogleDoc for ours to enumerate changes, e.g. https://docs.google.com/document/d/1HjZBnhPF3SQr3_Mt9wCMl625fH5hydifLp_Quo-3bTo/edit https://docs.google.com/document/d/1HjZBnhPF3SQr3_Mt9wCMl625fH5hydifLp_Quo-3bTo/edit.

Since our software (RSpace ELN) may be connected to different versions of Dataverse at various customer sites, updating to fix to work with 4.12 might then break interactions with 4.11. We'll look into your Kubernetes project, that would be an idea way to test against different versions.

Feel free to clone and run integration tests from https://github.com/IQSS/dataverse-client-java https://github.com/IQSS/dataverse-client-java, it would be good to have some forewarning if they break against a dev build of future Dataverse releases before it is released so we get some chance to modify our API client code.

Cheers Richard

On 8 Apr 2019, at 12:19, Philip Durbin notifications@github.com wrote:

@richarda23 https://github.com/richarda23 hi! I'm sorry to hear we may have broken something. I guess is that pull request #5655 https://github.com/IQSS/dataverse/pull/5655 could have changed something you're relying on. I haven't looked deeply at it. If you spot something, please let us know.

When you say Java SDK I assume you mean https://github.com/IQSS/dataverse-client-java https://github.com/IQSS/dataverse-client-java . It would be great to have some public continuous integration set up for it. It looks like we talked about this a bit at IQSS/dataverse-client-java#5 https://github.com/IQSS/dataverse-client-java/issues/5 . (I have some news I'll share on that issue.)

We don't publish any changelogs (we probably should, especially if you ask the https://keepachangelog.com https://keepachangelog.com/ people 😄 ). We do tag all issues and pull requests with GitHub milestones, if that helps. It's sometimes a lot to wade through.

IQSS generally only has the latest release in production and on our demo site. For public APIs like this I will admit that sometimes I poke around installations that are out there in the community. You can get a list from the bottom of https://dataverse.org/metrics https://dataverse.org/metrics I hope this helps. Thanks for the bug report!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/IQSS/dataverse/issues/5724#issuecomment-480790749, or mute the thread https://github.com/notifications/unsubscribe-auth/AVoDr7UQdEry7YB_VPJgzluhNM-kBcL-ks5veyXIgaJpZM4ch54D.

pdurbin commented 5 years ago

@richarda23 thanks, I like your Google doc idea for a change log. Seems lightweight.

Is DataverseOperationsTest.java the one with the failure? Can you tell which test is failing?

pdurbin commented 5 years ago

Wow, RSpace really can't integrate with Dataverse 4.12. I just clicked the "Test" button over at https://community.researchspace.com/apps and got this crazy error:

Screen Shot 2019-04-08 at 8 20 38 AM

Here's the text of the error:

Testing connection could not complete: Status: 500 - server error There was an error responding to this request, which has been logged.

Something went wrong: JSON parse error: Can not construct instance of com.researchspace.dataverse.entities.DataverseContacts: no String-argument constructor/factory method to deserialize from String value ('philip_durbin@harvard.edu'); nested exception is com.fasterxml.jackson.databind.JsonMappingException: Can not construct instance of com.researchspace.dataverse.entities.DataverseContacts: no String-argument constructor/factory method to deserialize from String value ('philip_durbin@harvard.edu') at [Source: sun.net.www.protocol.http.HttpURLConnection$HttpInputStream@632ff0f7; line: 1, column: 141] (through reference chain: com.researchspace.dataverse.entities.DataverseResponse["data"]->com.researchspace.dataverse.entities.Dataverse["dataverseContacts"]->java.util.ArrayList[0])

Ref: skAqKbtZlN

(Don't worry, I created my API token afterward since I posted it above.)

The next step would be to get a stack trace from a Dataverse server (server.log).

pdurbin commented 5 years ago

Hmm, actually, the 500 error must be on the RSpace side. I don't see anything in server.log on http://phoenix.dataverse.org which is currently running edf1f6d. Here's a screenshot:

Screen Shot 2019-04-08 at 8 26 20 AM
richarda23 commented 5 years ago

Yes, it could well be some fragility in our JSON parsing which is failing when it shouldn't do - have you made any changes to the model since 4.11?

On 8 Apr 2019, at 13:28, Philip Durbin notifications@github.com wrote:

Hmm, actually, the 500 error must be on the RSpace side. I don't see anything in server.log on http://phoenix.dataverse.org http://phoenix.dataverse.org/ which is currently running edf1f6d https://github.com/IQSS/dataverse/commit/edf1f6d0b9e1e55c130fce454416b30d74d76b67. Here's a screenshot:

https://user-images.githubusercontent.com/21006/55723916-3b0c9880-59d8-11e9-867c-9eaaac5d8f24.png — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/IQSS/dataverse/issues/5724#issuecomment-480810875, or mute the thread https://github.com/notifications/unsubscribe-auth/AVoDr-sNhYKcHqK3jNotRhO2pVzehnuiks5vezXngaJpZM4ch54D.

pdurbin commented 5 years ago

@richarda23 quite possibly but I haven't looked much yet. If you have a moment you can compare these:

Dataverse 4.11: https://dataverse.no/api/dataverses/root

Screen Shot 2019-04-08 at 10 05 38 AM

Dataverse 4:12: https://demo.dataverse.org/api/dataverses/demo

Screen Shot 2019-04-08 at 10 05 42 AM

I hope I'm giving you the right API endpoint to compare...

richarda23 commented 5 years ago

Thanks for the 2 models to compare Looks like dataverseContacts is populated in 4.12. Looking in our Java code, we have DataverseContact as having a field 'contactEmail': public class DataverseContacts { private String contactEmail; } Maybe this was a mistake on our part, and it is just exposed now that the dataverseContacts[] is populated, if in 4.11 it was always empty?

On 8 Apr 2019, at 15:07, Philip Durbin notifications@github.com wrote:

@richarda23 https://github.com/richarda23 quite possibly but I haven't looked much yet. If you have a moment you can compare these:

Dataverse 4.11: https://dataverse.no/api/dataverses/root https://dataverse.no/api/dataverses/root https://user-images.githubusercontent.com/21006/55730318-e5d78380-59e5-11e9-8dbf-ce34679aa25c.png Dataverse 4:12: https://demo.dataverse.org/api/dataverses/demo https://demo.dataverse.org/api/dataverses/demo https://user-images.githubusercontent.com/21006/55730328-ea9c3780-59e5-11e9-93da-3ff562a723e2.png I hope I'm giving you the right API endpoint to compare...

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/IQSS/dataverse/issues/5724#issuecomment-480846361, or mute the thread https://github.com/notifications/unsubscribe-auth/AVoDr3AvoVOWk8iPrA1-KEosnCo4xvXKks5ve00EgaJpZM4ch54D.

pdurbin commented 5 years ago

@richarda23 I just looked at pull request #5655 again but it's not immediately obvious to me what the intended change was. I also don't see any details in #5583. Standup is in 15 minutes so I'll ask then. I appreciate you digging in on this!

pdurbin commented 5 years ago

it's not immediately obvious to me what the intended change was

... which is exactly what an API changelog would help with, as @richarda23 mentioned above. Here's a screenshot from the API changelog for RSpace from https://docs.google.com/document/d/1HjZBnhPF3SQr3_Mt9wCMl625fH5hydifLp_Quo-3bTo/edit?usp=sharing

Screen Shot 2019-04-08 at 11 14 32 AM
richarda23 commented 5 years ago

OK, I've been digging around a bit. I found a Dataverse still running 4.11 - Heidelberg - with 46 sub data verses. All the ones I tried (about 10) had dataverseContacts set as an empty array.

In the Java library, I had modelled DataverseContact as a separate object:

public class DataverseContacts {
    private String contactEmail;
}

as this is how a POST to create a Dataverse is modelled :

{
  "name": "Scientific Research",
  "alias": "science",
  "dataverseContacts": [
    {
      "contactEmail": "pi@example.edu"
    },
    {
      "contactEmail": "student@example.edu"
    }
  ],

Using this DataverseContacts class seems to fail in 4.12 as the dataverseContacts is populated, but as simple string array, e.g.:

{
  "id": 4425,
  "alias": "rspace-demo",
  "name": "ResearchSpace Dataverse",
  "affiliation": "ResearchSpace",
  "dataverseContacts": [
    "richard@researchspace.com"
  ],
  "permissionRoot": true,
  "description": "A test Dataverse fo collaboration testing",
  "dataverseType": "ORGANIZATIONS_INSTITUTIONS",
  "ownerId": 1,
  "creationDate": "2016-07-13T09:08:44Z"
}

A unit test that exposed the error message reported by Philip earlier today seems fixed by using a simple List for dataverseContacts.

I will know tomorrow morning if the acceptance tests pass. We have a new release of RSpace out later this week which should fix this issue for community users. So sorry for the fuss, this was our incorrect interpretation of the Dataverse data model, which is just exposed now that you are populating the dataverseContacts[] in 4.12 Cheers Richard

On 8 Apr 2019, at 16:16, Philip Durbin notifications@github.com wrote:

it's not immediately obvious to me what the intended change was

... which is exactly what an API changelog would help with, as @richarda23 https://github.com/richarda23 mentioned above. Here's a screenshot from the API changelog for RSpace from https://docs.google.com/document/d/1HjZBnhPF3SQr3_Mt9wCMl625fH5hydifLp_Quo-3bTo/edit?usp=sharing https://docs.google.com/document/d/1HjZBnhPF3SQr3_Mt9wCMl625fH5hydifLp_Quo-3bTo/edit?usp=sharing https://user-images.githubusercontent.com/21006/55735601-a57d0300-59ef-11e9-905b-7d21712c65d8.png — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/IQSS/dataverse/issues/5724#issuecomment-480875470, or mute the thread https://github.com/notifications/unsubscribe-auth/AVoDr03uZCbo5fSF3EVZLKBdv4_D2wDBks5ve103gaJpZM4ch54D.

pdurbin commented 5 years ago

@richarda23 thanks for continuing to dig in on this. Heads up that I believe :ExcludeEmailFromExport controls whether or not dataverseContacts is populated or not. This is new as of Dataverse 4.12 and is documented (somewhat vaguely) at http://guides.dataverse.org/en/4.12/installation/config.html#excludeemailfromexport

Screen Shot 2019-04-08 at 2 21 15 PM

I brought this up at standup and @scolapasta is going to take a look. @matthew-a-dunlap worked on the code as well and probably knows more.

@richarda23 assuming all of the above is true, what would the Dataverse API changelog look like? Something like...

Dataverse 4.12

...?

Thanks.

matthew-a-dunlap commented 5 years ago

Thanks @pdurbin & @richarda23 for digging into this, and sorry for the breakage! The way we report those contacts has changed. We should probably unify our get and post methods around this, probably switching the get method to return the "contactEmail": as part of the contacts array.

I am in favor of capturing API changes per release. Maybe we'd create a page in the documentation that is updated for each release capturing the format changes?

scolapasta commented 5 years ago

Yes, I was looking into what changed and the format was (inadvertently?) changed from an array to a list (additionally there may have been a bug which is why in 4.11 the list shows as empty).

We'll discuss here what the correct format we want us - as @matthew-a-dunlap ponits out, probably array; if so, we'll revert back to that format.

richarda23 commented 5 years ago

Yes, that would be good, it would help people writing clients to know what updates they would have to make, some changes may be quite subtle (like this one) and only get exposed in certain conditions I've updated our Java client to use a DataversePost class and a DataverseGet class to deal with the different data structures for creation/get operations. If you decide on a common data structure for both in 4.13 then probably that would break our clientsSo ideally it would be good to get some idea of what's changed before you actually release the new version, so it's not a big panic for us when our client breaks in production - the idea Philip had about running our tests on a  pre-release build would also be good to get a bit of advance warning, also it's equally likely our client code is over-specified and not resilient to minor implementation changes in your API.Cheers  RichardOn 08 April 2019 at 19:42 matthew-a-dunlap notifications@github.com wrote: Thanks @pdurbin & @richarda23 for digging into this, and sorry for the breakage! The way we report those contacts has changed. We should probably unify our get and post methods around this, probably switching the get method to return the "contactEmail": as part of the contacts array.I am in favor of capturing API changes per release. Maybe we'd create a page in the documentation that is updated for each release capturing the format changes?—You are receiving this because you were mentioned.Reply to this email directly, view it on GitHub, or mute the thread.

pdurbin commented 5 years ago

@richarda23 thanks, I see the commit you're talking about: https://github.com/IQSS/dataverse-client-java/commit/ade973519d5e70fbb07b52c80f76d7c7be089fee

pdurbin commented 5 years ago

I just made pull request #5737 to add some tests and assertions around the JSON printing of Dataverse contacts (and other fields).

Here's what the test prints out and makes assertions on:

{
  "id": 42,
  "alias": "dv42",
  "name": "Dataverse 42",
  "affiliation": "42 Inc.",
  "dataverseContacts": [
    "dv42@mailinator.com"
  ],
  "permissionRoot": false,
  "description": "Description for Dataverse 42.",
  "dataverseType": "UNCATEGORIZED"
}
pdurbin commented 5 years ago

If you look at https://coveralls.io/builds/22680804/source?filename=src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java you can see that currently there are no tests for this code:

Screen Shot 2019-04-09 at 10 55 55 AM

pdurbin commented 5 years ago

Ok, in 50c0de8 I just added a commit to pull request #5737 to bring back the format we had in Dataverse 4.11:

{
  "id": 42,
  "alias": "dv42",
  "name": "Dataverse 42",
  "affiliation": "42 Inc.",
  "dataverseContacts": [
    {
      "displayOrder": 0,
      "contactEmail": "dv42@mailinator.com"
    }
  ],
  "permissionRoot": false,
  "description": "Description for Dataverse 42.",
  "dataverseType": "UNCATEGORIZED"
}
richarda23 commented 5 years ago

What are your plans for releasing this, at the moment we are about to release RSpace 1.58 later this week with a fix to work with DV4.12 as it is now, ie. assuming

"dataverseContacts": ["email@somewhere.com mailto:email@somewhere.com"] Cheers Richard

On 9 Apr 2019, at 17:31, Philip Durbin notifications@github.com wrote:

Ok, in 50c0de8 https://github.com/IQSS/dataverse/commit/50c0de8c1821f1c5524d34f8c8922a407dacb27b I just added a commit to pull request #5737 https://github.com/IQSS/dataverse/pull/5737 to bring back the format we had in Dataverse 4.11:

{ "id": 42, "alias": "dv42", "name": "Dataverse 42", "affiliation": "42 Inc.", "dataverseContacts": [ { "displayOrder": 0, "contactEmail": "dv42@mailinator.com" } ], "permissionRoot": false, "description": "Description for Dataverse 42.", "dataverseType": "UNCATEGORIZED" } — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/IQSS/dataverse/issues/5724#issuecomment-481325843, or mute the thread https://github.com/notifications/unsubscribe-auth/AVoDr5BhOocCy8-i5aCndgbhCzR40RH0ks5vfMBegaJpZM4ch54D.

pdurbin commented 5 years ago

Because we've been merging pull requests into develop for a while I made a second pull request with just the fix to restore the JSON format of dataverse contacts. That way, from the perspective of the master branch, it's a tiny "diff" compared to all the work we've done so far. So we can pick our poison. 😄

Please note that the hotfix branch above is for master. This will require a slightly different release process. I'd recommend reading through https://nvie.com/posts/a-successful-git-branching-model/#hotfix-branches and I'll include the part of section with a visual below.

In short, assuming that we want to release 4.12.1 with just the tiny diff, we would add additional commits to the branch behind pull request #5740 (5724-dv-contacts-hotfix) to bump version numbers, etc (most of http://guides.dataverse.org/en/4.12/developers/making-releases.html I guess) before merging into master. Then we would tag as normal. Then we would merge master into develop to pick up the bug fix in pull request #5740. Clear as mud? Let's talk about it. 😄

Screen Shot 2019-04-09 at 12 55 56 PM

@richarda23 hi! Your comment came in as I was typing all this. I'm concerned that other integrations will break as well. Thanks again for reporting this!

matthew-a-dunlap commented 5 years ago

@richarda23 Our plan is to unify our get and post methods and have the GET return this sort of syntax:

"dataverseContacts": [
    {
      "displayOrder": 0,
      "contactEmail": "dv42@mailinator.com"
    },     {
      "displayOrder": 1,
      "contactEmail": "otheremail@mailinator.com"
    }
  ],

Sorry for making y'all go back and forth on this. Is this a syntax your system can currently handle or something that can be easily change for your upcoming release?

richarda23 commented 5 years ago

Hi, That sounds fine, if we have time we'll try to make our code work with both using a custom derserisaliser, if not, it's straightforward for us to revert our code back once you have released the unified syntax.

Cheers Richard

On 9 Apr 2019, at 22:05, matthew-a-dunlap notifications@github.com wrote:

@richarda23 https://github.com/richarda23 Our plan is to unify our get and post methods and have the get return this sort of syntax:

{ "displayOrder": 0, "contactEmail": "dv42@mailinator.com" }

Sorry for making y'all go back and forth on this. Is this a syntax your system can currently handle or something that can be easily change for your upcoming release?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/IQSS/dataverse/issues/5724#issuecomment-481440180, or mute the thread https://github.com/notifications/unsubscribe-auth/AVoDr4mS3JhNG9_2njMvvVLG50N8cd-Hks5vfQC1gaJpZM4ch54D.

pdurbin commented 5 years ago

@richarda23 we're working on it. Please stay tuned.

All, in 50c5c2d I got rid of the complicated lambda that doesn't ever print dataverse contacts via API. Now it does (I added assertions) but I have mixed feelings about all this.

Pull request #5655 was for #5583 which was "email address of dataverse creator visible in API", which is basically about email privacy. There's a valid concern that installations of Dataverse are exposing the email addresses of contacts for datasets and dataverses. For example, if you go to https://dataverse.harvard.edu/api/dataverses/open-source-at-harvard you can see my email address:

Screen Shot 2019-04-10 at 10 58 04 AM

Because of this, I beefed up our documentation about email privacy a bit:

Screen Shot 2019-04-10 at 10 58 50 AM

However, I think we have this completely backward. I think Dataverse should preserve email privacy by default. Installations of Dataverse should have to opt-in to exposing the email addresses of their users. The Dataverse UI preserves the privacy of the email addresses of contacts. Out of the box, the Dataverse API does not preserve the privacy of the email addresses of contacts. Why?

pdurbin commented 5 years ago

@djbrooke now that pull request #5737 should we deploy the code to https://beta.dataverse.org so that @richarda23 can confirm that the RSpace integration with Dataverse will no longer be broken in our next release?

pdurbin commented 5 years ago

@richarda23 hi! Can you please run your tests or do whatever testing you'd like against https://dev2.dataverse.org ? I just deployed the latest from our develop branch (fe4dcfc) on it.

richarda23 commented 5 years ago

sorry, java is being a pain and not accepting your SSL cert for that domain, I'll have a look into this tomorrow

On 11 Apr 2019, at 17:33, Philip Durbin notifications@github.com wrote:

@richarda23 https://github.com/richarda23 hi! Can you please run your tests or do whatever testing you'd like against https://dev2.dataverse.org https://dev2.dataverse.org/ ? I just deployed the latest from our develop branch (fe4dcfc https://github.com/IQSS/dataverse/commit/fe4dcfce44b15a2d38c586d78df6deb3bc6dd240) on it.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/IQSS/dataverse/issues/5724#issuecomment-482188953, or mute the thread https://github.com/notifications/unsubscribe-auth/AVoDr6WJDD4PX2MK-gBU6K9Si_Jj49jAks5vf2PagaJpZM4ch54D.

pdurbin commented 5 years ago

@richarda23 whoops! Sorry. I just minted that SSL recently and I was playing a couple days ago with removing the intermediate cert (no browser warnings but I got a report that the cert was failing from Python: http://irclog.iq.harvard.edu/dataverse/2019-04-08#i_90463 ). Anyway, I put the intermediate certs back in so if you were seeing SSL errors like the ones below, please do try again. (These SSL errors disappeared for me once I fixed the cert on dev2.) Tomorrow or whenever is fine but heads up that I'm out tomorrow and all next week. If you need any assistance validating the fix in this issue, please reach out to support@dataverse.org . Thanks!

com.researchspace.dataverse.http.SearchOperationsIntegrationTest > testBasicSearchByTermOnly FAILED
    org.springframework.web.client.ResourceAccessException at SearchOperationsIntegrationTest.java:56
        Caused by: javax.net.ssl.SSLHandshakeException at SearchOperationsIntegrationTest.java:56
            Caused by: sun.security.validator.ValidatorException at SearchOperationsIntegrationTest.java:56
                Caused by: sun.security.provider.certpath.SunCertPathBuilderException at SearchOperationsIntegrationTest.java:56
richarda23 commented 5 years ago

Thanks, we reverted the Java SDK data model back to its 4.11 state and tests are passing on https://dev2.dataverse.org https://dev2.dataverse.org/ now.

On 11 Apr 2019, at 19:37, Philip Durbin notifications@github.com wrote:

@richarda23 https://github.com/richarda23 whoops! Sorry. I just minted that SSL recently and I was playing a couple days ago with removing the intermediate cert (no browser warnings but I got a report that the cert was failing from Python: ). Anyway, I put the intermediate certs back in so if you were seeing SSL errors like the ones below, please do try again. (These SSL errors disappeared for me once I fixed the cert on dev2.) Tomorrow or whenever is fine but heads up that I'm out tomorrow and all next week. If you need any assistance validating the fix in this issue, please reach out to support@dataverse.org mailto:support@dataverse.org . Thanks!

com.researchspace.dataverse.http.SearchOperationsIntegrationTest > testBasicSearchByTermOnly FAILED org.springframework.web.client.ResourceAccessException at SearchOperationsIntegrationTest.java:56 Caused by: javax.net.ssl.SSLHandshakeException at SearchOperationsIntegrationTest.java:56 Caused by: sun.security.validator.ValidatorException at SearchOperationsIntegrationTest.java:56 Caused by: sun.security.provider.certpath.SunCertPathBuilderException at SearchOperationsIntegrationTest.java:56 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/IQSS/dataverse/issues/5724#issuecomment-482246231, or mute the thread https://github.com/notifications/unsubscribe-auth/AVoDr6gkV3jgP07xNbheOoijv0iBOmJMks5vf4DrgaJpZM4ch54D.

pdurbin commented 5 years ago

@richarda23 fantastic! Thanks for running those tests! As I mentioned, I'd love to follow up with you in https://github.com/IQSS/dataverse-client-java/issues/5 to try to prevent breakages.

I tweaking the title of this issue a few days ago and it currently reads like this:

"Breaking API changes in 4.12: RSpace (and other?) integrations broken"

I've been thinking that maybe we could replace the word "breaking" with "surprising" or "unexpected". :smile: Maybe something like this:

"RSpace integration broken due to surprising API change"

(For the pull request, I eventually settled on giving it a title of "consistent format for dataverseContacts JSON".)

I say this because really what we've done for the next release (4.12.1) is emit JSON in the same format that we consume it for dataverse contacts. It was surprising that in 4.12 we started emitting JSON in an unexpected format. Mostly I'm just thinking about the title of the issue people will see and click when they browse closed issues linked from our release notes for 4.12.1. If you feel like tweaking the title a bit, please go ahead. Not a big deal. I really appreciate your jumping on this and even being willing to cut a release to accommodate the unexpected API change. I'm happy with how this has turned out. I like the consistency we implemented as a fix. And API users get more information (display order of dataverse contacts).