pepkit / pepdbagent

Database for storing sample metadata
BSD 2-Clause "Simplified" License
2 stars 1 forks source link

More Advanced API Naming, Tests, and Stability #20

Closed nleroy917 closed 2 years ago

nleroy917 commented 2 years ago

Overview

These changes are all-encompassing to the package and I was just trying to mature the API naming convention, build up some tests, and increase stability to handle edge cases, errors, etc. I also tried to create a consistent way of making database queries.

Most of my changes were made with pephub in mind. So, I tried to make returned results practical for something like that. If we need to be less specific, we can.

Changes

I made 3 main changes to the codebase:

  1. Standardize the API naming discussed in #18 We now have 4 main getters: get_project, get_projects, get_namespace, and get_namespaces. For the plural getters, they can accept no input or list input. For no input, the function will retrieve all available objects. For the list input, it will just return those objects named in the list. The singular functions will return just one instance of a project specified by a registry path (get_project) or str (get_namespace).

  2. I updated the internal sql query function For parameterized queries it was accepting an unspecified number of unnamed arguments, so I just changed it to accept a tuple as psycopg wants. There is some internal logic to map single values or a list to a tuple if not supplied properly.

  3. Unit tests With pytest I added some pretty basic unit test functionality. Was very helpful for testing my code and debugging. With this, I removed the big main block that was at the bottom of the pepagent.py file.

I also did some small things like rename an exception, moved constants around, and created some utils in utils.py.

Dependencies

I didn't add any dependencies, but we are heavily reliant on the development version of peppy, so before this can really get released, we probably need a release of peppy.

nleroy917 commented 2 years ago

Tests are failing because there is no database to test against. See here: #21

If anyone wants to help out here, that'd be awesome!