Rework auth/api_token parameters

[shoeffner]

Describe your environment

On host:

• OS: macOS, Sonoma 14.5, M1
• pyDataverse: this PR (i.e., main branch + patches)
• Python: 3.12
• Dataverse: 6.3 (local/container), 6.2 (demo.dataverse.org)

Inside container:

• OS: I think the python images use ubuntu, but didn't check
• pyDataverse: this PR (i.e., main branch + patches)
• Python: 3.11
• Dataverse: 6.3 (!) -- had to override DV_VERSION in docker-compose-test-all.yml to make tests pass, as three tests check for the version. See also Update DV_VERSION to 6.3 #197 .

Follow best practices

• Have you checked to ensure there aren't other open Pull Requests for the same update/change? make replace parameter work in edit_dataset_metadata #146 is partially affected by this as it changes the params argument of put_request.
• Have you followed the guidelines in our Contribution Guide? Yes, see Update the Contributor Guide #193 .
• 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?
  • This PR introduces custom authentication implementation to allow for API token and Bearer token-based authentication.
  • Sword requires BasicAuth, which this PR also addresses (see https://guides.dataverse.org/en/latest/api/sword.html#sword-auth)
  • An Auth implementation must be passed as a keyword-argument to the init function. We can discuss this requirement, but it felt better to be explicit about it and make sure implementers actually pass it correctly rather than accidentally passing it as the api_token or something.
  • I replaced some isinstance(..., ("".__class__, "".__class__)) with the much more legible and equivalent (as far as I can tell) isinstance(..., str).
  • It also deprecates the use of the previously ignored auth on individual functions in favor of generating multiple API objects with different auth methods.
    • For the deprecation, I introduced a DEPRECATION_GUARD to warn people who are trying to pass something to the functions. Initially I wanted to check for False, but there was one call defaulting to True (get_access) and this would not catch code explicitly setting auth=False on the callsite. So I decided to introduce a new default argument and show a warning if it is overwritten. Happy to discuss this decision – I have never done such an orderly deprecation before, so this is my first try :)
  • I updated the documentation.
  • Oh, and I smuggled a change to the Api.__str__ methods into the PR. As I scrolled by, I saw that they essentially could be removed and just the Api.__str__ method itself should use self.__class__.__name__ to get a similar result. However, the DataAccessApi will not print DataAccessApi: ... instead of the previous Data Access API: ... – the same applies for the others. However, this felt to be more in spirit with the docstring (which I then also updated slightly). If that's not so nice, we can move that to its own PR or even drop that commit entirely.
  • I had to adapt three unrelated things (conf.py, the self.base_url = None assignment in api.py, and the pyproject.toml) for mypy.
  • This PR almost accidentally also allows to pass params properly due to the name change of params -> headers.
• Why is this change required? What problem does it solve?
  • First and foremost, the issue it resolves is that tokens could potentially be leaked when sharing log messages, as the error response from dataverse includes the request URL, which, if the token is provided as ?key, contains the key.
  • The PR also lays the groundwork to allow for different authentication schemas in the future and even to configure it from the outside, favoring composition over inheritance. This will hopefully make maintenance and the refactoring in Refactor api.py into sub-modules #189 easier.
  • By deprecating the auth on individual methods, it will eventually be possible to remove this unused argument and make the API a little bit leaner and reduce confusing behavior.
• Screenshots (if appropriate)
• Put Closes #ISSUE_NUMBER to the end of this pull request

Testing

• Have you used tox and/or pytest for testing the changes? pytest
• Did the local testing ran successfully? yes, if applying Update DV_VERSION to 6.3 #197. For demo.dataverse.org, I could not run test_upload.py successfully, as I was unauthorized to perform those uploads (401). However, locally (outside the containers) I was able to run the asyncio tests with pytest-asyncio. The containers seem to miss this.
• 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).
  • I have only done this for 3e73fbf, as that is arguably the "essence" of this PR.

Others

• Is there anything you need from someone else? Take your time to review these changes. While some are quite repetitive and I already split some parts off into other PRs, this is a massive changeset due to the deprecation. In fact, maybe the deprecation should be its own PR? What do you think?

Documentation contribution

• [?] Have you followed NumPy Docstring standard? I hope so

Code contribution

• Have you used pre-commit? Yes, see also Run pre-commit run --all #196.
• Have you formatted your code with black prior to submission (e. g. via pre-commit)? Yes
• Have you written new tests for your changes? Yes
• Have you ran mypy on your changes successfully? Yes, but I had to add types-jsonschema to the lint dependencies and in the conf.py. I haven't tried --strict.
• Have you documented your update (Docstrings and/or Docs)? yes
• Do your changes require additional changes to the documentation? No, but m

Note that this PR depends on other PRs which should be reviewed and decided/acted upon first, I can also rebase/merge the changes into this branch afterwards.
In particular:

• Run pre-commit run --all #196
• Update DV_VERSION to 6.3 #197

Closes #192 .

[JR-1991]

Thanks, @shoeffner, that looks great! I agree with the changes made to allow passing either 'auth' or 'api_token'. I just tested it locally, and it works perfectly 🙌

[pdurbin]

I didn't run the code or even look at it very closely but I'm happy there are tests and the PR seems reasonable to me.

I'm leaving a few tiny suggestions for docs.

Thanks for the PR, @shoeffner! ❤️

[pdurbin]

Out of curiosity, where is this used?

I'm asking because upstream we've been trying to improve JSON Schema support:

• bklog: Deliverable - As a system integrator, I would appreciate a JSON Schema for validating my dataset JSON before uploading via API IQSS/dataverse-pm#26

[shoeffner]

In utils, jsonschema.validate is used:

pyDataverse/pyDataverse/utils.py

Line 8 in 3b6fe06

from jsonschema import validate

jsonschema is not typed, so mypy requires the type stubs for it. When you run mypy on the code you will get the respective error.

[shoeffner]

I had a look, the call chain is: pyDataverse.models.(Dataset|DVObject).validate_json -> pyDataverse.utils.validate_data -> jsonschema.validate.
There are also four schema.json files checked in at pyDataverse/schemas/json (which is not a package though, so one would need the filepaths to validate against them).

The function is not called automatically and supposed to be usable by consumers of the package, the validate_json functions are tested in test_datafile.py, test_dataset.py, and test_dataverse.py. The paths to the schemas are declared in the conftest.py.

[JR-1991]

@shoeffner, due to the merge of the jsonData pull request, this one has a couple of merge conflicts. I am happy to take care of those if you want.

[shoeffner]

I'll take a look, thanks for letting me know!

[shoeffner]

Alright, I rebased everything and resolved one incompatibility with #203, which I didn't pay attention to during the rebase. Should work now.

[JR-1991]

@shoeffner, thanks a lot - Merging!