Skip to content

Conversation

@juliemacdonell
Copy link

@juliemacdonell juliemacdonell commented Oct 21, 2025

Closes #123

What has been done to verify that this works as intended?

I have added the test class TestClientDirectCredentials which runs a simple integration test to verify the new authentication implementation succeeds. There is also a test which verifies the logic for when errors are expected to be raised.

Following the testing instructions, those tests passed. For the same reason as why the other test class in tests/test_client.py are skipped by default (quoted below), I've also skipped this test class by default.

For interactive testing, debugging, or sanity checking workflows, end-to-end tests are stored in tests/test_client.py. These tests are not run by default because they require access to a live Central server.

Why is this the best possible solution? Were any other approaches considered?

We also tested out a workaround on our end which was wrapping the Client in a context manager such that temporary files would be generated to dump credentials during use and then removed upon exit. This felt cumbersome in practice, and we have some concerns about this approach related to race conditions and security risks (see issue).

We don't see any significant downsides to allowing direct passing of base_url, username, and password into the instantiation of Client. One consideration is that multiple ways of authenticating could feel less straightforward for users of the library. This is hopefully mitigitated by the defensive implentation that a config file is used XOR credentials files are directly passed in.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

Intentional change:

  • Users can now initialize a Client directly with base_url, username, and password instead of providing a TOML config file.

Unchanged behavior:

  • File-based configuration continues to work exactly as before.
  • Default behavior (Client() using .pyodk_config.toml from the home directory) remains intact.

Regression risk:

  • Minimal. The new parameters are optional and only affect initialization when explicitly provided.

Do we need any specific form for testing your changes? If so, please attach one.

No specific form is needed; any Central instance with accessible credentials is sufficient to verify authentication. The tests can be run by removing the @skip decorator on the class.

Does this change require updates to documentation? If so, please file an issue here and include the link below.

I updated the README.md to show the new direct initialization option. Are there additional docs to be updated?

Issue link: #123

Before submitting this PR, please make sure you have:

  • included test cases for core behavior and edge cases in tests
  • run python -m unittest and verified all tests pass
  • run ruff format pyodk tests and ruff check pyodk tests to lint code
  • verified that any code or assets from external sources are properly credited in comments

@lindsay-stevens
Copy link
Contributor

Addressed in #126

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support passing authentication credentials directly into Client

2 participants