Skip to content

Epic/pysof#8

Closed
doug-helios wants to merge 46 commits into
mainfrom
epic/pysof
Closed

Epic/pysof#8
doug-helios wants to merge 46 commits into
mainfrom
epic/pysof

Conversation

@doug-helios
Copy link
Copy Markdown
Contributor

No description provided.

* feat/uv: complete MVP packaging docs and setup

* feat: add ci for python build

* feat: move the ci for python

* fix: update lock

* fix: update readme for macos and runners selfhosted

* tech debt: remove CI momentarly
* feat: first iteration of bindings

* feat: first binding completed

* feat: switch build to maturin

* feat: complete the core functions

* feat: add tests for content types

* docs: update readme for v1

* feat: multiple engine support
* feat: add pipe and script to build wheels locally

* feat:split pythonc ci steps

* fix:remove rust step

* fix: orchestrate workflow for ci at python

* fix: workflow release, removed PyPI for a bit

* tech debt: remove windows build temp

* fix: unify setup venv

* fix: split python ci/cd

* fix: remove deprecated pipes

* fix: optimize venv use

* fix: venv path

* fix: test coverage

* fix: add maturin

* fix: install maturin?

* fix: add rust toolchain

* fix: linter issues

* fix: linter target path

* fix: path for formats and linters

* fix: lint and checker target

* fix: last iter

* fix: args paths

* checkpoint 1
* feat: enable wheel artifact

* fix: remove restrictive branches, trigger on py-CI finish

* checkpoint?

* checkpoint pr

* Revert "checkpoint pr"

This reverts commit b6828d1.

* fix:add feture branch for testing

* change if for trigger
@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 13, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@doug-helios doug-helios requested a review from smunini September 15, 2025 18:09
Copy link
Copy Markdown
Contributor

@smunini smunini left a comment

Choose a reason for hiding this comment

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

Overall looks good with these comments. If someone clones this repo, but doesn't yet have uv installed, will the rust cargo build still succeed ok?

Comment thread .github/workflows/cd.yml
Comment thread .github/workflows/ci.yml Outdated
Comment thread .github/workflows/ci.yml Outdated
Comment thread crates/pysof/Cargo.toml Outdated
Comment thread crates/pysof/README.md Outdated
Comment thread crates/pysof/README.md Outdated
Comment thread crates/pysof/WHEEL_BUILDING.md
Comment thread crates/pysof/python-tests/__init__.py Outdated
@doug-helios
Copy link
Copy Markdown
Contributor Author

Overall looks good with these comments. If someone clones this repo, but doesn't yet have uv installed, will the rust cargo build still succeed ok?

I've updated to exclude it and added a note on how to build it at pysof/README.md

Comment thread .github/workflows/ci.yml Outdated
@doug-helios doug-helios marked this pull request as draft September 30, 2025 13:24
@smunini smunini closed this Sep 30, 2025
@smunini
Copy link
Copy Markdown
Contributor

smunini commented Sep 30, 2025

Abandoning in favor of multithread/pysof branch

@smunini smunini deleted the epic/pysof branch October 2, 2025 20:55
smunini added a commit that referenced this pull request May 19, 2026
The SoF v2 spec's content-negotiation table lists `application/octet-stream`
as the Parquet response MIME. sof-server was returning the
non-standard `application/parquet` on every Parquet response path. HFS
REST already emits the spec value; this brings sof-server in line.

- All three sof-server Parquet response paths
  (`crates/sof/src/handlers.rs` small-file direct return,
  `crates/sof/src/handlers.rs` standard processing, and
  `crates/sof/src/streaming.rs::stream_single_parquet_response` for
  large streaming) now emit:
    Content-Type: application/octet-stream
    Content-Disposition: attachment; filename="output.parquet"
  Matches the HFS REST byte-for-byte. (The multi-file ZIP wrapper
  already used `application/zip` and is unchanged.)
- `ContentType::from_string` accepts `application/octet-stream` as the
  spec input value for the `_format` parameter; `application/parquet`
  is kept as a permissive alias for back-compat with clients that
  still send the old shape.
- Doc-comment headers in handlers.rs and server.rs updated to list the
  spec MIME types.

Test:
- `test_parquet_response_uses_octet_stream_content_type` (integration)
  asserts the response Content-Type is `application/octet-stream`, the
  Content-Disposition names a `.parquet` file, and the body starts
  with the PAR1 magic.
smunini added a commit that referenced this pull request May 19, 2026
The CapabilityStatement-formats half of audit item #11 was already
closed in the audit #6/#7 sweep — `/metadata` now lists all four MIME
types sof-server actually serves. This commit closes the remaining
half: the SoF v2 `$sql-on-fhir-capabilities` endpoint that publishes
truthful supports* flags so clients can negotiate without trial and
error.

- New `GET /$sql-on-fhir-capabilities` route wired in `server.rs`,
  backed by `sof_capabilities` handler in `handlers.rs`.
- Returns a `Parameters` resource matching HFS REST's shape so clients
  can use the same response decoder against either binary. For a
  stateless server the truthful values are:
    supportsViewDefinitionRun     = true
    supportsViewDefinitionExport  = false
    supportsSqlQueryRun           = false
    supportsInDbRunner            = false
    supportsRelativeReference     = false
    supportsCanonicalReference    = false
    supportsAbsoluteReference     = false
    supportedFormat               = ndjson, json, csv, parquet

- Test stub in `common.rs` mirrors the production handler.
- Fix collateral test failure: `test_run_view_definition_ndjson_output`
  was still asserting the pre-audit-#8 `application/ndjson` content-type
  on the NDJSON response. Updated to the spec value
  `application/x-ndjson` (production has emitted this since #8).

Test:
- `test_sof_capabilities_endpoint` asserts the endpoint returns
  `application/fhir+json`, the right resource shape, every expected
  boolean (with truthful values for a stateless server), and all four
  `supportedFormat` codes.
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.

3 participants