[WIP] feat: add artifact and job repositories#1207
Draft
keithmanville wants to merge 5 commits intodevfrom
Draft
[WIP] feat: add artifact and job repositories#1207keithmanville wants to merge 5 commits intodevfrom
keithmanville wants to merge 5 commits intodevfrom
Conversation
364441b to
c60c7da
Compare
This commit fixes a bug in which the REST API could raise a 500 error when a queue attached to an entrypoint was deleted and then the entrypoint was modified. The fix is to unlink all entrypoints from a queue when it is deleted. The implementation adds a new repository utility function, `unlink_parents`, that performs the unlinking for any parent/child relationship, so that it can be used in other repositories in the future. It also adds the `get_one_resource` utility function, which is useful in cases where it is necessary to assert a resource exists and the resource object is needed. It can be used to refactor other repository code for a simpler design pattern that has the benefit of not querying the database multiple times. This commit includes a new integration test that exhibits the previously failing behavior. It adds new repository utility tests for the new utility functions. It updates the queue deletion test to assert that no parents exist after deletion.
This commit updates the `unlink_child` repository utility function to use `get_one_resource`. This refactor makes the code simpler, avoids repeated queries to the database, and removes the needs for asserts that were being used to satisfy the type checker.
This commit adds the entrypoint repository, which is modeled after patterns in the existing
repositories. It updates the service layer to use the new data access pattern. It adds a new test
suite for the entrypoint repository and adds additional test coverage to the existing entrypoint
integration tests.
It adds the following new repository utility functions:
- get_exact_latest_snapshots: similar to get_latest_snapshots, but raises an error if any snapshots
are not found.
- get_one_resource: similar to get_one but for resources
- get_one_snapshot: similar to get_one but retrieves a specific snapshot by ID instead of the latest
- create_resource_children: similar to set_resource_children, but operates on resources that are not
yet in the database
- unlink_children: removes all children of the provided type
It updates some patterns to prefer performing existence checks in data retrievel functions, instead
of performing existence checks first and then retrieving. This reduces the number of queries since
existence checks also involve querying the database. Examples of this include increased reliance on
get_one (and the new get_one_resource and get_exact_latest_snapshots).
It updates UnitOfWork to use a context manager to handle commiting changes to the database (and
rolling back if an error occurs).
It adds a UnitOfWorkService base class, which handles the injection of the UnitOfWork to a service
class. Once all repositories are completed, no other services should be cross-injected into
eachother, instead all services will access other entities via the UnitOfWork object.
It removes the conditional error (error_if_not_found bool being passed) and the conditional commits
(commit bool being passed) in the services where possible.
It updates the experiments service to use the entrypoint repository instead of injecting entrypoint
services, and use the new UnitOfWorkService base class and the UnitOfWork context manager.
It uses EntityType instead of passing strings where possible, which required refactoring some
repository tests.
It makes the following changes for parameter validation:
- moves validation of entrypoint parameters from the service to schema layer
- adds a new error for input parameter validation
- updates the plugin service layer to use the new error (moving this logic to the schema layer is
left as a future task)
It makes deleted entrypoints accessible through GET /entrypoints (via the showDeleted query
parameter) and GET /entrypoints/{id} to make the behavior consistent with other repositories. It
also updates the frontend to display deleted entrypoints.
It adds an error handler for EntityDeletedError and changes the response code to 423 LOCKED to
differentiate it from NOT_FOUND. This error is raised when attempting to modify a deleted resource.
c60c7da to
62ab4e8
Compare
This commit adds the artifact and job repositories, which are modeled after patterns in the existing repositories. It updates the service layers to use the new data access pattern. It adds a new test suite for the artifact and job repositories. It makes external services (mlflow and redis) accessible by injecting those services into UnitOfWork instead of injecting them into other services directly.
62ab4e8 to
95443a8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.