pyDataverse icon indicating copy to clipboard operation
pyDataverse copied to clipboard

Update DV_VERSION to 6.3

Open shoeffner opened this issue 1 year ago • 4 comments

Version 6.3 is what the :unstable docker image currently provides.

All Submissions

Inside container:

  • [x] OS: I think the python images use ubuntu, but didn't check
  • [x] pyDataverse: this PR (i.e., main branch + patch)
  • [x] Python: 3.11
  • [x] Dataverse: 6.3 (!) -- this is what this PR does

Follow best practices

  • [x] Have you checked to ensure there aren't other open Pull Requests for the same update/change? yes, basically all due to the API version change.

  • [x] Have you followed the guidelines in our Contribution Guide? Yes, but it seems a little outdated. Will create a follow-up issue for this one to update it.

  • [x] Have you read the Code of Conduct? Yes.

  • [x] Do your changes in a separate branch. Branches MUST have descriptive names.

  • [x] Have you merged the latest changes from upstream to your branch? Yes Describe the PR

  • [x] What kind of change does this PR introduce?

    • Updates DV_VERSION from 6.2 to 6.3
  • [x] Why is this change required? What problem does it solve?

    • the :unstable docker image got updated
  • [x] Screenshots (if appropriate) image

  • [x] Put Closes #ISSUE_NUMBER to the end of this pull request

Testing

  • [x] Have you used tox and/or pytest for testing the changes? pytest, sh run-tests.sh
  • [x] Did the local testing ran successfully? yes
  • [ ] Did the Continuous Integration testing (Travis-CI) ran successfully?

Commits

  • [x] Have descriptive commit messages with a short title (first line).
  • [x] Use the commit message template
  • [x] Put Closes #ISSUE_NUMBER in your commit messages to auto-close the issue that it fixes (if such).

Others

  • [ ] Is there anything you need from someone else?

Documentation contribution

  • [ ] Have you followed NumPy Docstring standard?

Code contribution

  • [x] Have you used pre-commit?
  • [x] Have you formatted your code with black prior to submission (e. g. via pre-commit)?
  • [ ] Have you written new tests for your changes?
  • [ ] Have you ran mypy on your changes successfully?
  • [ ] Have you documented your update (Docstrings and/or Docs)?
  • [ ] Do your changes require additional changes to the documentation?

Closes #195

shoeffner avatar Jul 18 '24 23:07 shoeffner

I wonder if it is better to use jq to grab the version from the service, which is similar to how it is done for API_TOKEN. Something along these lines:

export DV_VERSION=$(curl -s http://dataverse:8080/api/info/version | jq -r '.data.version')

This way, we wouldn't risk being "out of sync" whenever there is a new version. What are your thoughts?

JR-1991 avatar Jul 19 '24 07:07 JR-1991

@JR-1991 yes, good idea. Let's grab the version from the live system. It's more future proof.

pdurbin avatar Jul 19 '24 18:07 pdurbin

I am not sure what the best course of action is here. In general, the /vX/ path parameter should be used to handle versioning, but it appears that the versions returned by /api/info/version track breaking changes much better, as /v1/ seems to have been static from what I can tell, without having too much insights into the project.

It all depends on what the stance on API-level support for pyDataverse is -- i.e., should every version in general just support whatever API was available at that time, or should it also support older API levels for some time. In the former case, yes, getting the DV_VERSION from the API is sensible -- or the asserts could even be removed or relaxed to be more like "is a version present". However, this might also cause tests to fail without having an idea as to why they fail. In this case, I knew that if something unrelated would break, it might also be API-Version related. If the version would be selected from the API itself, I wouldn't know this.

If the goal is to support differently versioned Dataverse instances, it would even be more difficult, but one could either select a version with DV_VERSION or auto-detect the API level using the API.

I will have to think about this a little bit more, just some thoughts for now. For a short-term fix, I think fetching it from the live system or keeping it as-is to detect changes early on is good.

shoeffner avatar Jul 19 '24 19:07 shoeffner

@shoeffner wow, good questions.

I defer to @JR-1991 on this.

For now I think we just want the test to pass. 😄

@shoeffner you're very welcome to copy and paste your thoughts into the #python channel at https://chat.dataverse.org if you like! 😄

pdurbin avatar Jul 19 '24 19:07 pdurbin