Skip to content

feat(controller): add a uri property to every file controller - #158

Open
LukasGold wants to merge 2 commits into
mainfrom
feat/file-controller-uri
Open

feat(controller): add a uri property to every file controller#158
LukasGold wants to merge 2 commits into
mainfrom
feat/file-controller-uri

Conversation

@LukasGold

@LukasGold LukasGold commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Closes #68.

Stacked on #148, which makes InMemoryController constructible. Merge that first, this base retargets to main automatically.

Changes

  • FileController.uri: the location of the file in the URI scheme of its storage backend. Concrete, returns None by default.
  • LocalFileController.uri: file://, via Path.absolute().as_uri(), see https://en.wikipedia.org/wiki/File_URI_scheme.
  • RemoteFileController.uri: the stored url. S3FileController and WikiFileController inherit it, so they report s3://... and https://.../wiki/File:... without an override.
  • InMemoryController.uri: None.
  • tests/test_file_controller_uri.py: 7 offline tests.

Rationale

A caller holding a FileController had no way to ask where the file actually is. Each subclass kept that in a different shape: a Path, a url, or nothing at all.

WikiFileController already builds the page url in its url property (src/osw/controller/file/wiki.py:167), and S3FileController parses self.url for its bucket and key, so putting uri on RemoteFileController covers both from what they already carry.

uri is a read interface: derived on every access, never stored, so it cannot go stale. Each controller keeps the handle its backend actually needs and answers uri in its own scheme. The wider harmonization of path / url / uri, and a from_uri() write-side counterpart, are proposed in #68 (comment) and are out of scope here.

Notes

  • The property is deliberately not abstract. pydantic v1's ModelMetaclass extends ABCMeta, so @abstractmethod is enforced at instantiation and an abstract uri would break every file controller outside this package that predates it. A concrete None default keeps this a feat, not a breaking change.
  • InMemoryController returns None rather than raising. A stream is a legitimate file source, it just has no address.

Verification

Offline suite passes (66 passed, 1 skipped). The wiki and S3 controllers need a live backend, so their uri is covered through RemoteFileController and, for the wiki page url, by calling the property directly on a stand-in.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Release preview

Merging this PR would release v2.1.0 (current: v2.0.2).

Changelog preview (truncated)
## v2.1.0 (2026-09-02)

### Bug Fixes

- Allow uploading a file from an in-memory stream
  ([`64d99d6`](https://github.com/OpenSemanticLab/osw-python/commit/64d99d66ea45bc56bdaa6d12f2502df0bbbaf88c))

### Features

- **controller**: Add a uri property to every file controller
  ([`5ec4038`](https://github.com/OpenSemanticLab/osw-python/commit/5ec4038ee8e7dc1085c4ec7d1c290667791bee48))

### Refactoring

- Make FileController.uri a concrete default instead of abstract
  ([`ad43d07`](https://github.com/OpenSemanticLab/osw-python/commit/ad43d078c4d8fe4341907358bb4cc0a36a51432d))

### Testing

- Rename oold.py to oold_test.py so its tests are collected
  ([`20072a9`](https://github.com/OpenSemanticLab/osw-python/commit/20072a9249cd97126a222c62a70f84e0433343ef))

Preview via python-semantic-release and conventional commits.

- file:// for LocalFileController, via Path.as_uri
- the stored url for RemoteFileController, so s3:// and https:// follow
- None for InMemoryController, a stream has no location
@LukasGold
LukasGold force-pushed the feat/file-controller-uri branch from 998e846 to 5ec4038 Compare September 2, 2026 12:35
@LukasGold
LukasGold changed the base branch from main to fix/in-memory-file-upload September 2, 2026 12:35
- returns None on the base class, so controllers outside this package
  that predate uri stay instantiable (ModelMetaclass extends ABCMeta)
- document uri as the derived read interface for a location
- state the implicit url contract on RemoteFileController
@LukasGold

Copy link
Copy Markdown
Contributor Author

Changed uri from an abstract property to a concrete one returning None (ad43d07).

Reason: pydantic v1's ModelMetaclass extends ABCMeta, so @abstractmethod on an osw model is enforced at instantiation. Verified against main, where a file controller defining only get and put constructs fine. With the abstract version of this PR the same class raises:

TypeError: Can't instantiate abstract class Downstream without an implementation
for abstract method 'uri'

That is a breaking change for any file controller outside this package, and it was not marked as one, so it would have shipped as a minor release. tests/test_file_controller_uri.py::test_a_controller_that_predates_uri_still_works pins the behaviour.

None is also the honest default: before this PR no controller had a uri at all, so a subclass that does not override it is in exactly the state it was in before, and InMemoryController shows None is a real answer rather than a placeholder.

Follow-up on harmonizing path / url / uri across the controllers, plus a from_uri() counterpart: #68 (comment).

@LukasGold LukasGold left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reasonable changes that provide a uniform access point

Base automatically changed from fix/in-memory-file-upload to main September 2, 2026 15:45
@LukasGold
LukasGold requested a review from SimonStier September 2, 2026 15:59
@LukasGold LukasGold self-assigned this Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add property uri to every file controller

1 participant