Enh/2/behavior sessions 2

[mochic]

Addresses #2 and #3.

Uses a different pattern in behavior_session which I'd like to port over to other fetching/writing methods and also partially add to the core client so that table references can be resolved by the core client code.

Also, I'd like to return all records fetched from all fetching code and maybe have the return thresholding as some client level option (ideally to fetch less via the query params if that exists on the slims api ) or for that to be on the user also returning a list of validated and unvalidated data like fetch_behavior_session_content_events

Also, we should change the way Record name aliases are serialized/deserialized for better naming but that might be better for a separate pr.

[mochic]

Can't add reviewers, if either of y'all could take a look at this it would be helpful. @dyf @patricklatimer @jtyoung84 @bruno-f-cruz

[patricklatimer]

Let's talk it through, but I now kind of think the client.fetch_models should commit to returning validated models, then you don't have to handle the multiple return types all over.

[patricklatimer]

I might name this mouse_fk (or mouse_pk?) or something, mouse_name implies it'll match the barcode or labtracks id rather than the pk.

[mochic]

Addressed in 74e32b8

[patricklatimer]

I think it would be better to accept either a SlimsMouseContent or the parameters to fetch it (i.e. barcode)

[mochic]

I think that fetching logic for other records shouldn't be embedded in other fetch functions when necessary, it creates unnecessary coupling within our fetching functions. This function is designed to be used like this:

fetch_behavior_session_content_events(client, fetch_mouse_content(client, labtracks_id), ...)

I would prefer to use some record reference abstraction in the future over this pattern but adding record references seems like it goes outside of the bounds of this pr. The references would allow us to centralize and control when pks get resolved, how they get resolved, and also minimize an end user's experience with that.

[patricklatimer]

Now that I think about it a little more, I don't think this method should return two lists. Doing that kind of splits the difference between using models and using dicts, and I think if we want to use the models we should fully commit.

Might be useful to think through the use cases of when model validation might fail, because ideally the database has proper data (especially if it's been written from the aind_slims_api).

1. Data got fed to the database manually through ui or through some other process (not this library), and our models are more restricted than the SLIMS fields.
2. The models got updated and old data doesn't match
3. The database schema got updated and the models don't match.

I think these are all things that we'd want to catch with integration test errors, but shouldn't expect users to have to deal with.

Anyway, maybe more discussion is warranted, but I'm now thinking validation errors should raise exceptions (maybe we'd want to raise one error for all the failures like Pydantic does; maybe TypeAdapter is appropriate?)

[patricklatimer]

model._slims_table should be a str, right?
model._slims_table,

[mochic]

I think this is a consequence of the way SlimsBaseModel has it's table definition defined, which we should probably amend but in a separate pr.