Refactored API code into subpackage

[ErikBjare]

The idea is to split the big resources.py file into smaller files, like the code examples for flask-restplus.

[johan-bjareholt]

Why are you working on refactoring aw-server-python when aw-server-rust has feature parity and will be replacing it?

[ErikBjare]

Because it is still the reference implementation, and faster to prototype stuff in.

[johan-bjareholt]

But it's just refactorization, it doesn't add any functionality? Why spend time on refactoring something that we don't intend to keep for a longer period of time?

[ErikBjare]

Because I wanted it to follow the coding conventions of the flask-restplus example projects, which will make it easier for me to follow the examples here (JWT) and here (OAuth2) as well as make me properly learn how to structure flask-restplus projects in general.

[TimeToBuildBob]

🤖 AI code review

This PR refactors the aw-server API code into a subpackage: it renames aw_server/rest.py to aw_server/resources/rest.py, updates imports in server.py and init.py to use the new module path, and adds an empty aw_server/resources/init.py. It also changes setup.py to use find_packages() instead of a hardcoded package list, and updates Pipfile to use editable local paths for aw_core and aw-server instead of a git reference.

Not safe to merge — 1 P1 open

Confidence 3/5

⚠️ 1 generated file excluded and NOT reviewed: Pipfile.lock (+51/-13 lines, 4KB). Machine-generated output is removed from the diff before review — from what the model sees and from the size gate alike — so nothing below says anything about it.

3 findings · ❌ 1 P1 · ⚠️ 2 P2

2 inline

❌ P1 high — Pipfile:7

The Pipfile change replaces the aw_core git reference with an editable local path ../aw-core. This is a development-only dependency specification that will break for anyone who clones the aw-server repository without also having a sibling aw-core checkout at that exact relative path. The original aw-core = {ref = "master", git = "https://github.com/ActivityWatch/aw-core.git"} was self-contained and installable from a fresh clone. With this change, pipenv install will fail with a FileNotFoundError if ../aw-core does not exist. This is a contract break for contributors and CI environments that do not manually clone aw-core first. The PR description does not mention this as an intentional workflow change, and it is not part of the refactoring goal. The same applies to the aw-server = {editable = true, path = "."} entry, which is redundant because the project itself is already the package being installed, but that is less harmful.

Keep the git reference for aw_core, or document that a sibling aw-core checkout is required.

_{How this was verified: Checked the diff: the original line was aw-core = {ref = "master",git = "https://github.com/ActivityWatch/aw-core.git"}; the new line uses a relative path that requires a sibling checkout.}

⚠️ P2 medium — aw_server/__init__.py:11

The import change in aw_server/init.py from from . import rest to from . import resources is correct for the new package layout, but the empty aw_server/resources/__init__.py does not re-export rest. Code that previously did from aw_server import rest (or from . import rest inside the package) will now break because aw_server.rest no longer exists. The PR moves the module but does not provide a compatibility shim. Any external caller or plugin that imports aw_server.rest directly will get an ImportError. The PR description says the goal is to split the big resources.py file, but this PR only moves it; a compatibility import in aw_server/__init__.py (e.g., from .resources import rest) would preserve the old API. Without it, this is a breaking change for any code that relied on the old module path.

Add `from .resources import rest` to aw_server/__init__.py to keep the old import path working.

_{How this was verified: Searched the repository for references to aw_server.rest or from . import rest; the only in-repo references were in server.py and init.py, both updated, but external callers are not visible in this diff.}

Comments outside the diff

_{These could not be anchored to a line GitHub accepts, so they appear only here.}

⚠️ P2 medium — aw_server/resources/rest.py:35

The CustomJSONEncoder.__init__ method calls super().__init__() without passing through *args and **kwargs. This means any arguments passed to the constructor (e.g., sort_keys=True, indent=2) are silently dropped. Flask's app.json_encoder is instantiated with default arguments in most cases, but if a caller or a future change passes options, they will be ignored. This is a pre-existing bug in the moved file, not introduced by this PR, but the PR is the opportunity to fix it. The observable consequence is that JSON serialization options are not honored, which could affect pretty-printing or key ordering in API responses. However, since the current code never passes arguments, the impact is currently nil. This is a low-severity latent bug.

super().__init__(*args, **kwargs)

_{How this was verified: Checked the class definition in aw_server/resources/rest.py lines 33-45. Flask's JSON encoder is instantiated with app.json_encoder and may pass configuration. The init signature accepts *args and **kwargs but discards them.}

1 advisory finding (summary-only, not scored)

These P2 guard, heuristic, trade-off, or documentation claims are retained for judgment without opening review threads.

⚠️ P2 medium — setup.py:26

The setup.py change replaces the hardcoded packages=['aw_server'] with packages=find_packages(), which will include any subpackage that has an init.py. The new aw_server/resources/init.py is empty, so find_packages() returns ['aw_server', 'aw_server.resources']. However, the check if packages != ["aw_server", "aw_server.resources"] only prints a warning and does not fail the build, so if a stray package directory (e.g., a tests/ or a leftover aw_server/rest.py directory) is present, it will be silently included in the distribution. More importantly, the PR description says the goal is to split resources.py into smaller files, but this PR only moves the file; the empty init.py and the find_packages() change are preparatory. The actual defect is that the warning is non-fatal, so a packaging mistake (like accidentally including a top-level package) will not be caught at build time, and the resulting wheel may contain unexpected modules. This is a low-impact issue because the current tree only has the two expected packages, but the check is ineffective as a guard.

if set(packages) != {"aw_server", "aw_server.resources"}:
    raise RuntimeError(f"Unexpected packages found: {packages}")

_{How this was verified: Read setup.py lines 25-27. The condition only prints a message; it does not raise an error. The list order from find_packages() is not guaranteed to be sorted, so the comparison may be unreliable.}

Files changed (6) — the diff as I read it

• Pipfile — Changes aw_core from a git ref to an editable local path and adds aw-server as an editable local path.
• aw_server/__init__.py — Changes the import from .rest to .resources.
• aw_server/main.py — Moves the logger creation from module level into the main() function.
• aw_server/resources/rest.py — Renames aw_server/rest.py to this path and changes relative imports to absolute aw_server imports.
• aw_server/server.py — Updates imports to use aw_server.resources.rest instead of .rest.
• setup.py — Switches from a hardcoded packages list to find_packages() and prints a warning if extra packages are found.

_{Also changed, not described above: aw_server/rest.py}

_{Reviewed 88d3d7bb649e · openrouter/deepseek/deepseek-v4-flash-0731 · llm engine · 61s · about this reviewer}

_{Maintainer commands}

_{@TimeToBuildBob review (own line) — fresh review · @TimeToBuildBob fix — a worker acts on the findings. Once per comment; 👀 = received.}

[TimeToBuildBob]

❌ P1 — The Pipfile change replaces the aw_core git reference with an editable local path ../aw-core. This is a development-only dependency specification that will break for anyone who clones the aw-server repository without also having a sibling aw-core checkout at that exact relative path. The original aw-core = {ref = "master", git = "https://github.com/ActivityWatch/aw-core.git"} was self-contained and installable from a fresh clone. With this change, pipenv install will fail with a FileNotFoundError if ../aw-core does not exist. This is a contract break for contributors and CI environments that do not manually clone aw-core first. The PR description does not mention this as an intentional workflow change, and it is not part of the refactoring goal. The same applies to the aw-server = {editable = true, path = "."} entry, which is redundant because the project itself is already the package being installed, but that is less harmful.

[TimeToBuildBob]

⚠️ P2 — The import change in aw_server/init.py from from . import rest to from . import resources is correct for the new package layout, but the empty aw_server/resources/__init__.py does not re-export rest. Code that previously did from aw_server import rest (or from . import rest inside the package) will now break because aw_server.rest no longer exists. The PR moves the module but does not provide a compatibility shim. Any external caller or plugin that imports aw_server.rest directly will get an ImportError. The PR description says the goal is to split the big resources.py file, but this PR only moves it; a compatibility import in aw_server/__init__.py (e.g., from .resources import rest) would preserve the old API. Without it, this is a breaking change for any code that relied on the old module path.