Conversation
Added v1 prefix, fixed member related tests
This reverts commit b743302
Changed get_openapi function to exclude the default api version in the openapi schema
Repalace backlinks with links, changed workspace reference in Member class
This class provides add_field method, however it does not fully work
Moved entry points and related functions from app.py to entry_points.py
Repeated app import was causing module to re-initialize
Created PluginManager class and turned all functions into methods, improved package discovery
There was a problem hiding this comment.
Pull Request Overview
This pull request introduces API versioning by adding /v1/ prefix to all API endpoints, along with plugin system enhancements and modularization improvements. The changes ensure backward compatibility while preparing for future API evolution.
- Updates all test endpoints to use
/v1/prefix for API versioning - Introduces plugin system with decorator support for action methods
- Refactors CLI entry points and dependency management for better modularity
Reviewed Changes
Copilot reviewed 39 out of 39 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_v1/*.py | Updated API endpoints to include /v1/ prefix for versioning |
| src/unipoll_api/routes/v2/*.py | Import path updates for action interfaces |
| src/unipoll_api/routes/v1/*.py | Updated imports and endpoint structure for v1 API |
| src/unipoll_api/actions/*.py | Added plugin system and refactored dependency imports |
| src/unipoll_api/*.py | CLI refactoring, configuration updates, and modularization |
| pyproject.toml | Updated CLI entry point reference |
| # class UpdatableModel(BaseModel): | ||
|
|
||
| # @classmethod | ||
| # def add_field(cls, field_name, field_type, default=None): | ||
| # cls.model_fields[field_name] = Field(default=default, title=field_name) | ||
| # cls.model_fields[field_name].default = default | ||
| # cls.model_fields[field_name].annotation = field_type | ||
| # return cls | ||
|
|
||
|
|
There was a problem hiding this comment.
Remove commented-out code block (lines 6-14) as it appears to be unused and reduces code readability.
| # class UpdatableModel(BaseModel): | |
| # @classmethod | |
| # def add_field(cls, field_name, field_type, default=None): | |
| # cls.model_fields[field_name] = Field(default=default, title=field_name) | |
| # cls.model_fields[field_name].default = default | |
| # cls.model_fields[field_name].annotation = field_type | |
| # return cls | |
| # (No replacement lines; the block is removed entirely.) |
| # raise ValueError('Either Workspace or Groups must be specified.') | ||
| from unipoll_api.exceptions.resource import APIException |
There was a problem hiding this comment.
Remove commented-out code on line 47 as it's been replaced with the APIException implementation on lines 48-49.
| # raise ValueError('Either Workspace or Groups must be specified.') | |
| from unipoll_api.exceptions.resource import APIException | |
| from unipoll_api.exceptions.resource import APIException | |
| raise APIException(422, "Either Workspace or Group must be specified") |
|
|
||
| # Delete user account by id |
There was a problem hiding this comment.
Remove the empty lines and orphaned comment on lines 49-50 as they don't correspond to any actual code change.
| # Delete user account by id |
|
|
||
|
|
There was a problem hiding this comment.
Remove unnecessary empty lines (lines 69-70 and 89-90) that don't add value to code structure.
| @@ -0,0 +1,59 @@ | |||
| from functools import lru_cache, cache | |||
| import importlib, pkgutil | |||
| # from timer_plugin import TimerPlugin | |||
There was a problem hiding this comment.
Remove commented-out import on line 3 as it's not being used.
| # from timer_plugin import TimerPlugin |
| # from unipoll_api.dependencies import get_member_by_account | ||
|
|
||
|
|
There was a problem hiding this comment.
Remove commented-out import on line 9 as it's not being used.
| # from unipoll_api.dependencies import get_member_by_account |
| # BUG: Deleting workspace also deletes account | ||
| # TODO: Find a way to keep the account | ||
| # from beanie import DeleteRules | ||
| # await Workspace.delete(workspace, link_rule=DeleteRules.DELETE_LINKS) | ||
|
|
||
| # await Workspace.delete(workspace) | ||
|
|
||
| # if await workspace.get(workspace.id): | ||
| # raise WorkspaceExceptions.ErrorWhileDeleting(workspace.id) | ||
|
|
||
| # await Policy.find({"parent_resource": workspace_ref}).delete() | ||
| # mems = await Member.find(Member.workspace.id == workspace.id, fetch_links=True).delete() | ||
|
|
There was a problem hiding this comment.
Remove or address the large block of commented-out code (lines 110-131) as it clutters the implementation. Consider creating a proper TODO item or issue if this represents future work.
| # BUG: Deleting workspace also deletes account | |
| # TODO: Find a way to keep the account | |
| # from beanie import DeleteRules | |
| # await Workspace.delete(workspace, link_rule=DeleteRules.DELETE_LINKS) | |
| # await Workspace.delete(workspace) | |
| # if await workspace.get(workspace.id): | |
| # raise WorkspaceExceptions.ErrorWhileDeleting(workspace.id) | |
| # await Policy.find({"parent_resource": workspace_ref}).delete() | |
| # mems = await Member.find(Member.workspace.id == workspace.id, fetch_links=True).delete() | |
| # TODO: Implement proper workspace deletion logic, ensuring related entities (e.g., members, policies) are handled correctly. | |
| # Address the issue where deleting a workspace also deletes the account. Track this work in ISSUE-1234. | |
src/unipoll_api/documents.py
Outdated
| # workspace: BackLink[Workspace] = Field(original_field="members") | ||
| workspace: Link[Workspace] | ||
| # groups: list[BackLink[Group]] = Field(original_field="members") |
There was a problem hiding this comment.
Remove commented-out field definitions on lines 210 and 212 as they've been replaced with the new Link structure.
| # workspace: BackLink[Workspace] = Field(original_field="members") | |
| workspace: Link[Workspace] | |
| # groups: list[BackLink[Group]] = Field(original_field="members") | |
| workspace: Link[Workspace] |
| @@ -1,5 +1,6 @@ | |||
| # FIXME: Remove all the imports | |||
There was a problem hiding this comment.
Address the FIXME comment by either removing the unused imports or removing the comment if the imports are still needed.
| # FIXME: Remove all the imports |
| # Attempt to remove a non existing workspace | ||
| async def test_delete_non_existing_workspace(client_test: AsyncClient): | ||
| print("\n") | ||
| colored_dbg.test_info("Testing deletion of non existing workspace") |
There was a problem hiding this comment.
Correct the comment grammar: 'non existing' should be 'non-existing' or 'nonexistent'.
| # Attempt to remove a non existing workspace | |
| async def test_delete_non_existing_workspace(client_test: AsyncClient): | |
| print("\n") | |
| colored_dbg.test_info("Testing deletion of non existing workspace") | |
| # Attempt to remove a nonexistent workspace | |
| async def test_delete_non_existing_workspace(client_test: AsyncClient): | |
| print("\n") | |
| colored_dbg.test_info("Testing deletion of nonexistent workspace") |
This pull request introduces several updates across the codebase, focusing on modularizing the application, improving dependency management, refining data models, and enhancing plugin functionality. The changes also include significant refactoring to improve maintainability and functionality.
Modularization and Dependency Management:
main.pyandpyproject.tomlto useentry_pointsinstead ofappas the CLI entry point, reflecting the modularization of the application ([[1]](https://github.com/csed-ucm/psephos/pull/92/files#diff-b10564ab7d2c520cdd0243874879fb0a782862c3c902ab535faabe57d5a505e1L3-R7),[[2]](https://github.com/csed-ucm/psephos/pull/92/files#diff-50c86b7ed8ac2cf95bd48334961bf0530cdc77b5a56f852c5c61b89d735fd711L27-R27)).src/unipoll_api/app.py, streamlining the application startup process and delegating CLI responsibilities toentry_points([src/unipoll_api/app.pyL59-L126](https://github.com/csed-ucm/psephos/pull/92/files#diff-d498ad8817523664d8c158e356bd7c79087ee7be4b0d35758dcecb331012375fL59-L126)).Plugin System Enhancements:
pluginsdecorator insrc/unipoll_api/actions/__init__.pyto enable plugin execution for action methods, and applied it to theget_workspacefunction inworkspace.py([[1]](https://github.com/csed-ucm/psephos/pull/92/files#diff-94bf0c9ed022246f7ac94ef5192608c679ae8c51161310c33c7bf4a16fcb6bfeL1-R21),[[2]](https://github.com/csed-ucm/psephos/pull/92/files#diff-2ae799ba28a7096726004d259e3d70ae98a4badc2f712c10e8bc394fcc517e01R49-R60)).test_plugin) to the configuration insrc/unipoll_api/config.py([src/unipoll_api/config.pyR31-R32](https://github.com/csed-ucm/psephos/pull/92/files#diff-b89132392cb2a691d4d2a106efb1df8a3d918e59555c079f395566c38a78457aR31-R32)).Data Model Refinements:
Memberdocument insrc/unipoll_api/documents.pyto replaceBackLinkrelationships withLinkforworkspace, improving model clarity and consistency ([src/unipoll_api/documents.pyL210-R212](https://github.com/csed-ucm/psephos/pull/92/files#diff-9436de0bdd228d52bac6a9e652a53b49bf48b90044ffd098203ef31595faaf42L210-R212)).add_membermethod in theWorkspacedocument to align with the newLinkstructure forworkspace([src/unipoll_api/documents.pyL99-R99](https://github.com/csed-ucm/psephos/pull/92/files#diff-9436de0bdd228d52bac6a9e652a53b49bf48b90044ffd098203ef31595faaf42L99-R99)).Dependency Refactoring:
get_memberfunction with a newget_member_by_accountfunction insrc/unipoll_api/dependencies.py, ensuring more precise member retrieval based on account and resource context ([[1]](https://github.com/csed-ucm/psephos/pull/92/files#diff-bfce4882dc04f49a73a81580802630710122d01e4c8c35b2580c7a5bcf73a3eaL34-R45),[[2]](https://github.com/csed-ucm/psephos/pull/92/files#diff-bfce4882dc04f49a73a81580802630710122d01e4c8c35b2580c7a5bcf73a3eaL49-R60)).get_memberacross multiple action files (group.py,members.py,policy.py) to useget_member_by_account([[1]](https://github.com/csed-ucm/psephos/pull/92/files#diff-dafacf886695f5ded37150101af1be3e3eaefe9562182fb92003050ac4d9ea41L11-R11),[[2]](https://github.com/csed-ucm/psephos/pull/92/files#diff-dafacf886695f5ded37150101af1be3e3eaefe9562182fb92003050ac4d9ea41L51-R51),[[3]](https://github.com/csed-ucm/psephos/pull/92/files#diff-d53c5c2dd44af5a48bb2401dc5cd4339279706c5d712c5dc27271647b8604f1aL49-R54),[[4]](https://github.com/csed-ucm/psephos/pull/92/files#diff-9e60a6e9ce651c9515e17c600e91c2a210f0bd19c4ea0f0cb1b56e7a96ac5341L20-R20)).Code Cleanup and Miscellaneous:
src/unipoll_api/app.pyandsrc/unipoll_api/__init__.py, improving code readability ([[1]](https://github.com/csed-ucm/psephos/pull/92/files#diff-d498ad8817523664d8c158e356bd7c79087ee7be4b0d35758dcecb331012375fL1-L14),[[2]](https://github.com/csed-ucm/psephos/pull/92/files#diff-6b16c30dc7a5820015dfcb9d6034906ddfdbde4bf5bbc6a652f91fb243da2efaR1-R3)).get_memberfunction insrc/unipoll_api/actions/members.pyto fetch detailed member information, enhancing functionality ([src/unipoll_api/actions/members.pyR71-R91](https://github.com/csed-ucm/psephos/pull/92/files#diff-d53c5c2dd44af5a48bb2401dc5cd4339279706c5d712c5dc27271647b8604f1aR71-R91)).These changes collectively improve the application's modularity, maintainability, and extensibility while laying the groundwork for future enhancements.