AP-564_extracting-tind-client

[jason-raitz]

I'll use this PR to address comments and suggestions from the Jira ticket.

comments & suggestions to address:

• use uv for actions and readme? (Yes! ✅ )
• only worry about python v3.13?
  • how about >= v3.13 ❓
  • v3.14 to version matrix in ci ✅
• tools should check ., not just tind_client.
  • I think . is a bit too broad because of hidden files/dirs
  • How about we just do tind_client & tests? ( implemented ❓ )
• Why are we disabling C0114 in pylint? ( idk? removed ✅ )
• TIND s/ILS/DA/g (TODO) Done! ✅
• why switch to pytest? where are missing tests from willa? (idk. TODO: investigate)
• Do we want merges to main to generate a ‘head’ package that can be consumed for testing?
  • We'll eventually want to have a publish action to pypi, right?
  • TODO: investigate
• ~Since we're going with a client pattern, we don't need to support direct functional calls to the API, it makes it a bit messy.
  • TODO: make direct functions private and DRYer (remove repetitive auth params) DONE! ✅
• Why do all the fetch_* APIs take a * parameter?
  • I think this was an artifact I forgot to clean up when experimenting with function calls in development.
  • TODO: investigate/remove This is a python keyword separator. basically, anything after the * must include the key and not rely on position. I removed all but one of these. as it makes sense to use it in the _search_request for pagination.

[awilfox]

Few small nits, but overall good so far.

[awilfox]

Is this truly a change if there hasn't been a release yet?

[jason-raitz]

Yeah, good point. I'll move any changes up to the added category before marking ready for review.

[jason-raitz]

I've updated the TODO list at the top of the PR. Only remaining tasks for this ticket are:

• refactor to use unittest instead of pytest
• implement builds
  • build/publish action
  • setup pypi publishing

[awilfox]

Looking much nicer and cleaner now, I like where this is heading. 👍🏻

[awilfox]

It's not a good idea to specify the value like this here, because that means any changes to the environment after the code is loaded won't be reflected. For instance, if code loads environment variables from Docker Secrets or an .env file.

>>> import os
>>> def test(foo: str = os.environ.get("LANG", "")):
...     print(foo)
...     
>>> test()
en_US.UTF-8
>>> os.environ['LANG']
'en_US.UTF-8'
>>> os.environ['LANG'] = 'en_GB.UTF-8'
>>> os.environ['LANG']
'en_GB.UTF-8'
>>> test()
en_US.UTF-8

Better to do:

api_key: str = ''

and then in the method body:

self.api_key = api_key or os.environ.get(...)

[awilfox]

This is good, but just to be really clear, it might be nice to have an environment variables subsection like we do in the Willa readme for easy reference.