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

Update DV_VERSION to 6.3 #197

Merged
merged 1 commit into from
Aug 22, 2024
Merged

Update DV_VERSION to 6.3 #197

merged 1 commit into from
Aug 22, 2024

Conversation

shoeffner
Copy link
Collaborator

@shoeffner shoeffner commented Jul 18, 2024

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

All Submissions

Inside container:

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

Follow best practices

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

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

  • Have you read the Code of Conduct? Yes.

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

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

  • What kind of change does this PR introduce?

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

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

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

Testing

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

Commits

  • Have descriptive commit messages with a short title (first line).
  • Use the commit message template
  • 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

  • Have you used pre-commit?
  • 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

This is what the :unstable docker image currently provides.

Closes gdcc#195.
Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. We did release 6.3. Approved.

@shoeffner shoeffner mentioned this pull request Jul 19, 2024
30 tasks
@JR-1991 JR-1991 self-assigned this Jul 19, 2024
@JR-1991 JR-1991 added the pkg:testing test related activities label Jul 19, 2024
@JR-1991
Copy link
Member

JR-1991 commented Jul 19, 2024

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?

@pdurbin
Copy link
Member

pdurbin commented Jul 19, 2024

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

@shoeffner
Copy link
Collaborator Author

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.

@pdurbin
Copy link
Member

pdurbin commented Jul 19, 2024

@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! 😄

@JR-1991 JR-1991 added this to the 0.3.4 milestone Aug 21, 2024
@JR-1991 JR-1991 merged commit 8445a09 into gdcc:main Aug 22, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:testing test related activities
Projects
Development

Successfully merging this pull request may close these issues.

dataverse:unstable increased the version to 6.3
3 participants