Merge dev into main: reconcile 3 months of divergence since the June revert - #153
Merged
Conversation
schema and noofentry in prompt datasets queries
no of entries for file formats other than csv
Initialize y_axis_columns list for y-axis configurations.
New top-level entity (internal name Publication, UI label Resource): - Publication model: UUID/slug (counter-dedupe), typed metadata columns (authors, publication_date, license reusing DatasetLicense, external_source_link), status, owner FKs, resource_type FK, sectors/ geographies M2M, download_count, query-shape indexes. - PublicationBlock: ordered file-XOR-youtube blocks with a DB CheckConstraint. - ResourceType: admin-managed flat lookup (adapted from Sector, +is_active). - PublicationStatus / PublicationBlockType enums. - UseCase/Collaborative gain a publications M2M; publications added to all four UC/Collab input exclude lists (no draft-linking bypass window). - seed_resource_types management command (idempotent). - Layer 1 model tests; test settings disable the ES signal processor so DB tests stay hermetic.
- type_publication.py: TypePublication/Block/ResourceType with fields enumerated (never __all__), plus filter/order. - Publication permission classes in authorization/permissions.py: create, change, delete, publish (name-based roles) and AllowPublishedPublications read gate — centralized, keyed on publication_id, individual-owner branch kept, no share-model fallback. - publication_service.py: metadata validation, create/update/status helpers, org/owner/anonymous scoping with include_public union, server-enforced pagination bounds. - publication_schema.py flow: create/update/publish/unpublish/delete + status-gated list/detail via MutationResponse envelope; registered in schema. - Layer 3/4 tests: CRUD, role gating, publish, cross-org denial, draft read-gating, org-scoped and anonymous listings.
…wnload - youtube.py: extract/validate YouTube video ids across watch/youtu.be/embed. - publication_uploads.py: server-side file validation (extension allow-list, 50 MB cap, PDF magic-byte sniff). - publication_blocks.py: add file/youtube block, replace file (deleting old bytes), remove + contiguous renumber, reorder, read-time access gate. - publication_download_view.py + URL: gated block-file serve (draft private, published public, download_count increment, PDF inline). - publication_signals.py: post_delete removes a block's file from disk. - Block mutations (add file/youtube, replace, remove, reorder); permission resolver maps a block id to its parent publication. - Layer 2/1/3/4 tests across youtube, uploads, block CRUD, download gate. Also annotates a pre-existing untyped local in dynamic_chart_view (surfaced by mypy once urls.py imports the views package) to keep the mypy gate green.
- publication_document.py: PublicationDocument indexing real columns only (title/description/status/resource_type/sectors/geographies/owner/dates); should_index_object gates on PUBLISHED so drafts never index; explicit related_models = [Organization, User, ResourceType, Sector, Geography] with get_instances_from_related so a renamed Resource Type/sector/geo re-indexes. - search_publication.py: /api/search/publication/ (AllowAny) with resource_type/sector/geography facet filters. - Unified search: publication branch in index names, result normalization, type-count aggregation, default type list, and serializer fields. - publication_signals.py: publish adds / unpublish + delete drop the search document (draft never searchable); ES errors logged and swallowed. - Settings ES index name; search route. - Security: youtube URL validator now rejects non-http(s) schemes (a matching host on javascript: could otherwise be stored — stored-XSS vector). - Tests: index-decision, re-index mapping (incl. ResourceType), signal predicate, and the javascript-scheme regression.
…ratives - publication_linking.py: only-PUBLISHED-is-linkable guard + published-filter helper (Resources are the first non-Dataset linkable entity). - UC & Collaborative schemas gain add/remove/update_*_publications trios (DRAFT-guard on the parent, only-published on the resource). - TypeUseCase/TypeCollaborative expose a published-only 'publications' render field so an unpublished/deleted resource silently drops out and reappears on re-publish. - TypePublication exposes linkedUsecases/linkedCollaboratives/linkedCount for the owner's 'linked to N' flag. - Tests: link guard, stale-link (both UC and Collab), linked-count, cross-org link affordance.
- dataspace_sdk/resources/publications.py: search (REST ES endpoint), plus get_by_id / list_all / get_organization_publications / create / update / delete over GraphQL (Resources have no REST write API). - Registered in client.py __init__ and set_organization (org header scoping). - tests/test_publications.py + a wiring assertion in test_client.py.
The UC/Collab publication link trios previously guarded only on the container's DRAFT status — like the pre-existing dataset trio — so any authenticated user could change another org's use case / collaborative links. Add assert_can_manage_links (owner, or org member with can_change; superuser always) to all six link mutations. The intentional cross-org affordance is unaffected: authorization is on the container, not the linked resource, so org B's use case may still link org A's published resource. Adds an IDOR regression test.
- api/schema/publication_architecture.md: full backend design — submodules,
helpers, data model, indexes, security, six-layer test map, cross-repo
overview, limitations.
- tests/journeys/publications/{create-blocks-publish,link-unpublish-relink}.py:
on-demand Layer 5 scripts.
…lt, N+1) - B1 (cross-tenant leak): the linkedUsecases/linkedCollaboratives/linkedCount fields on TypePublication are now gated to owner/org (`_caller_can_see_links`) and filtered to PUBLISHED projects, so a public resource no longer exposes another org's private draft project titles. - B2 (discovery): add 'publication' to the unified-search get() default type list (the cache-key default already had it — the two now match). - B4 (N+1 + plan DoD): get_scoped_publications prefetches resource_type/organization/user + sectors/geographies/blocks; the blocks resolver relies on Meta ordering to reuse the prefetch cache; added an assertNumQueries listing test. - O1: reject an explicit-empty required field on update (don't blank title). - O2: regression test that the plain useCase update can't attach a resource. B3 (org-header trust in the list resolver) is a platform-wide pattern mirrored from the dataset/aimodel resolvers — flagged in the arch doc for a shared- middleware fix, not patched per-endpoint. Adds tests/test_publication_qc_fixes.py.
The frontend metadata form needs to populate a Resource Type dropdown; expose an active-only, name-sorted resourceTypes query. Adds a test.
health_check previously returned JsonResponse(data) unconditionally, so the ECS container healthcheck (curl -f) and any future smoke gate would false-green a container with a dead DB/ES/Redis/telemetry connection. Also exposes git_sha so a deploy can verify the running code matches what was just pushed.
Follows the same ARG->ENV pattern the container already uses for other build-time config. Feeds health_check's new git_sha field.
Generating migration files at deploy time instead of using committed ones is unsafe under a rolling deployment (briefly 2 tasks live) and means the schema that lands in prod was never reviewed. migrate itself stays here for now; moving it to an explicit one-off step is next.
github.event.head_commit.modified is only populated for single-commit pushes -- a squash-merge touching aws/cloudformation would silently skip infra sync. The CFN deploy is already idempotent (--no-fail-on-empty-changeset), so simplest robust fix is to just run it every time; costs one extra ~10-20s idempotent call per deploy.
if: always() made this job run even if the workflow was cancelled before it started. Now that deploy-infrastructure always runs (prior commit), it can never legitimately be 'skipped' either, so the implicit needs: gating (success-only, false on cancellation) is exactly the behavior wanted -- no explicit if: needed.
Bumps aws-actions/amazon-ecs-deploy-task-definition v1 -> v2 (purely additive per its changelog) to use its built-in run-task support: runs a standalone task on the new task definition, waits for it to stop, and fails the whole action on a non-zero container exit -- before the service update ever happens. Network config (subnets, security groups, public-IP assignment) is read from the currently running service via describe-services rather than hardcoded, so the migration task always runs in the same network context as the app.
mkdir/chmod/touch on api/migrations existed only to let the runtime makemigrations step (removed earlier) write new migration files. migrate doesn't need to write to that directory, just read committed migrations, so this block has been dead weight since makemigrations was dropped.
Adds a smoke-tests job calling CivicDataSpace-test's run-smoke.yml, passing deployed_sha (github.sha) so the gate verifies /health/'s git_sha field matches, and min_passed to catch a fully-skipped run looking green. Requires a new repo variable DEV_API_BASE_URL and the same secrets DataSpaceFrontend's own pipeline already passes to this workflow (HOME_URL_DEV, TEST_EMAIL_1/2, TEST_PASSWORD_1/2). Also captures the currently-running task definition ARN before the service update (deploy-app now has an output for it) -- needed by the rollback job that follows in the next commit.
Restores the task definition ARN captured before this deploy's service update (previous commit), waits for the rolled-back service to stabilize, then exits 1 -- the run stays red even after successful mitigation, matching the policy that a rollback is damage control, not a pass. Migrations applied by the bad deploy are never auto-reverted; the error message points at where to find what ran. OTel collector rollback is explicitly out of scope here -- separate service, independent risk, keeps this change's blast radius to the app service only.
settings.py gives TELEMETRY_URL a hardcoded otel-collector default even when the env var is unset, so a deployment that deliberately runs no collector (as this one does) still probed a host that cannot resolve and logged an ERROR on every health check -- roughly every 30s from the compose healthcheck alone, forever, for something optional. Now reports "not_configured" without probing. Does not touch the required_services gate, so this still cannot affect the status code either way.
Real regression from the image-based deploy. The old compose bind-mounted the whole working tree (`.:/code`), which incidentally gave the app access to user-uploaded media on the host. Removing that mount (correctly -- it would shadow the deployed image) left nothing serving MEDIA_ROOT, so /code/files did not exist in the container at all: 2246 files on the host, 0 visible to the app. Every file-field lookup then raised FileNotFoundError -- e.g. an organization logo's `size` -- which errored the GraphQL queries the dashboard depends on and left it blank. The doubled-looking path in those errors (/code/files/public/files/public/organizations/...) is not a bug; that nested layout genuinely exists on the host, since MEDIA_ROOT is BASE_DIR/files/public and the stored names start with files/public/. Mounts only ./files:/code/files -- persistent data, never the whole tree. Verified live: 2246 files now visible in the container and zero FileNotFoundError after exercising the publishers page.
AuthClient built every Keycloak URL as
`{keycloak_url}/auth/realms/{realm}/...`, so the SDK could only ever
talk to a Keycloak served under /auth. Against a Keycloak served at the
domain root every token request 404s, which is not recoverable through
configuration -- there is no value of keycloak_url that avoids it.
Adds `keycloak_base_path`, defaulting to "/auth" so existing callers
are byte-for-byte unaffected. Pass "" for a root-hosted Keycloak. The
path is also skipped when keycloak_url already ends with it, so callers
passing a full base URL do not get it emitted twice.
Verified against both live servers: the composed URLs return 401 (the
endpoint exists, credentials rejected) while the old composition returns
404 on a root-path server.
fix(sdk): stop hardcoding /auth in Keycloak URLs
…lient #129 added keycloak_base_path to AuthClient, but DataSpaceClient -- the class every consumer actually imports -- never passed it through. No caller of the public API could set it: DataSpaceClient(..., keycloak_base_path="") silently behaved identically to omitting it, because AuthClient just used its own default. Found while attempting the ParakhAI dev flip -- the fix in #129 was real but incomplete. Adds a test asserting DataSpaceClient actually forwards the value, not just that AuthClient accepts it, so this class of gap fails a test next time.
…-path fix(sdk): DataSpaceClient did not forward keycloak_base_path to AuthClient
run-smoke.yml now preflights its Keycloak configuration instead of letting the authenticated API tests skip silently and the run go green on nothing. It needs the client secret to do that. `dataspace` is a confidential client, so the ROPC token request returns 401 unauthorized_client without it. The reusable workflow declares the secret optional so this repo kept parsing before this change, but api-smoke fails its preflight until the secret is passed and set.
…smoke ci: pass KEYCLOAK_CLIENT_SECRET to the smoke workflow
Django runs sync views under ASGI via sync_to_async(thread_sensitive=True), which executes them on one shared thread per process. With a single uvicorn worker that means exactly one sync request is processed at a time, however many arrive. Measured on dev: 12 concurrent POSTs to /api/auth/keycloak/login/ returned at 4s, 7s, 10s, 14s, 17s, 20s, 23s, 26s, 29s, 32s and 36s - near-perfect ~3s increments, each queued behind the last. That queue is what exhausted Postgres. Every in-flight request holds a connection while it waits its turn: 20 concurrent calls took the connection count from 6 to 26, one per request. Deep enough queues reached max_connections (100) and Postgres began refusing with "FATAL: sorry, too many clients already". Requests that waited past nginx's 60s proxy timeout became 504s, refused ones became 500s, and the deploy pipeline failed too because manage.py could not get a connection either. Workers are processes, so N workers give N concurrent sync requests and the queue drains N times faster. The work is I/O-bound on Keycloak, so this helps well past the box's 2 CPUs. --limit-concurrency is the backstop: in-flight requests are capped at workers x limit (60), which stays under max_connections with headroom for other clients. Excess requests get a fast 503 rather than queueing until the database runs out of slots. Shedding load is recoverable; exhausting connections takes the deploy pipeline down with it. Both are env-tunable so a box can be sized without a code change.
KeycloakLoginView introspected the token twice per request - once inside validate_token and again for roles and organizations - and validate_token then called userinfo as well. On this deployment the client lacks the scope for userinfo, so that call returns 403 every time and falls through to a branch that rebuilds the same fields from the introspection response it already had. A guaranteed failing network call on every login, roughly a third of the request's latency. validate_token now accepts an introspection the caller already has, and skips userinfo when introspection already carries sub plus an identifier. The userinfo path remains for deployments where introspection is sparse, so behaviour is unchanged where it was actually doing work. This matters beyond latency: the request holds a database connection for its whole duration, so every round-trip removed is connection-hold time removed, which is what ran the pool dry.
/health/ returned {"database": "healthy"} in 0.44s while Postgres was
refusing new connections with "FATAL: sorry, too many clients already",
the login endpoint was failing, and the deploy pipeline could not run
manage.py.
The check ran SELECT 1 on the request's own connection. That connection
is already established, so it keeps answering however saturated the
server is. The endpoint could not fail for the condition that was
actually taking the service down - so a green health check was worse
than no health check, and anything gating a deploy or paging on it stayed
green throughout.
Now also opens a fresh connection and closes it immediately. The two
failure modes are independent, and it is the second that catches
exhaustion.
Measured before setting a number rather than reaching for (2 x CPU) + 1. The container uses 740MB resident with one worker and the host has about 2.4GB available, so four workers risked an OOM - a worse outage than the connection exhaustion this is fixing. Two workers land near 1.3GB and match the 2 CPUs. In-flight requests then cap at 30, comfortably under max_connections (100) with room for other clients. Two workers alone would only double throughput, but removing two of the three Keycloak round-trips per login cuts per-request time as well, and the two compound.
…exhaustion fix: stop Postgres connection exhaustion (serialized sync views, duplicate Keycloak calls, blind health check)
This is the actual cause of the connection exhaustion. My earlier fix
(more uvicorn workers) addressed a real constraint but not this one, and
concurrency did not improve as a result - the bottleneck is a row lock
in Postgres, which no number of application workers can help.
Captured from pg_stat_activity during a burst of concurrent logins:
36 active | Lock | tuple
31 active | Lock | transactionid
16 idle in transaction | Client| ClientRead
67 UPDATE "ds_user" SET "password" = ..., "last_login" = ...
Every login called user.save() unconditionally, rewriting every column
of the same row. Concurrent logins for one user therefore queued on that
row's lock, and each waiting request held a database connection while it
waited - which is what walked the connection count up to max_connections
and produced "sorry, too many clients already", the 504s, and the failed
deploy.
Two changes:
- sync_user_from_keycloak compares against the stored values and saves
only when a field actually changed, with update_fields to keep the
UPDATE narrow. A repeat login of an unchanged user now performs no
write at all, so there is no lock to contend on.
- validate_token no longer calls user.save() on the Django-JWT path. It
rewrote an unchanged row on every authenticated request for no benefit.
New-user creation is untouched: that INSERT is genuine work.
…ention fix: stop rewriting the user row on every login (the actual cause of connection exhaustion)
The image is 14.1GB and `docker pull` now runs past the deploy step's 40 minute command_timeout, so deploys fail outright. That is what stopped the row-lock fix in #136 from reaching dev - it merged green and then could not be deployed. Where the size comes from, measured in the running container: 12.1GB RUN pip install -r requirements.txt 4.3GB site-packages/nvidia 1.7GB site-packages/torch 592MB site-packages/triton That is roughly 6.6GB of CUDA runtime on a 2-CPU EC2 instance with no GPU, which cannot execute any of it. Installing CPU-only torch first means the pinned torch==2.9.0 is already satisfied and pip never reaches for the CUDA build. PEP 440 treats the local version segment as compatible, so 2.9.0+cpu satisfies ==2.9.0 and requirements.txt needs no change. The wheel was confirmed to exist for this exact version and platform (torch-2.9.0+cpu-cp310-cp310-manylinux_2_28_x86_64.whl), so this changes the build of torch, not the version. --no-cache-dir on both installs drops the pip wheel cache from the layer. This is a prerequisite now rather than an optimisation: nothing else can deploy until the pull fits inside the timeout.
fix: install CPU-only torch so the image can actually be deployed
Image size is a deploy-time failure mode here and nothing surfaced it. The image reached 14.1GB - 6.6GB of it CUDA runtime on a GPU-less box - and docker pull then ran past the deploy step's 40 minute command_timeout, so deploys failed with "Run Command Timeout" and no indication of the cause. Builds stayed green throughout; the cost only appeared on the host, an hour later, on an environment that could no longer be deployed to. Fails the build instead, so the feedback lands on the PR that caused it. The ceiling is 8GB against a current 3.56GB - set from the measured size after the CPU-only torch fix, with room for ordinary growth but not for another multi-gigabyte dependency arriving unnoticed. The error message names the usual culprits so whoever hits it knows where to look.
ci: fail the build if the image exceeds a size ceiling
DEFAULT_THROTTLE_RATES was set to {"anon": "100/hour", "user":
"1000/hour"} and had no effect whatsoever: DEFAULT_THROTTLE_CLASSES was
never configured, no view declares throttle_classes, and the endpoint
that actually gets hammered (/api/graphql) is a Strawberry view, not a
DRF one.
It was not merely inert. While diagnosing a flood of 429s on 2026-09-03
it was the first thing found, read as the cause, and very nearly
"fixed" - a change that would have altered nothing while sending the
investigation the wrong way.
Replaced with a comment pointing at api/middleware/rate_limit.py, which
is what actually runs: 5000/hour for GET, 1000/hour for other methods,
keyed on the client IP from X-Forwarded-For.
No behaviour change - the setting was doing nothing.
…nfig chore: remove the dead DRF throttle config that misled a diagnosis
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+232
to
+257
| name: Smoke Tests | ||
| needs: deploy | ||
| # No `environment:` here -- GitHub rejects the entire workflow file at | ||
| # parse time if a `uses:` job declares one. Consequence: this job also | ||
| # cannot see environment-scoped vars, which is why api_base_url comes | ||
| # from a repo-level var. | ||
| uses: CivicDataLab/CivicDataSpace-test/.github/workflows/run-smoke.yml@CI | ||
| with: | ||
| api_base_url: ${{ vars.DEV_API_BASE_URL }} | ||
| # An obviously-wrong sentinel SHA fails the reusable workflow's own | ||
| # deployed-SHA-vs-live-/health/ assertion on purpose, for the | ||
| # force_smoke_failure test path. | ||
| deployed_sha: ${{ (inputs.force_smoke_failure == true && 'forced-failure-sentinel') || github.sha }} | ||
| min_passed: ${{ inputs.force_smoke_failure && 999 || 1 }} | ||
| secrets: | ||
| HOME_URL_DEV: ${{ secrets.HOME_URL_DEV }} | ||
| TEST_EMAIL_1: ${{ secrets.TEST_EMAIL_1 }} | ||
| TEST_PASSWORD_1: ${{ secrets.TEST_PASSWORD_1 }} | ||
| TEST_EMAIL_2: ${{ secrets.TEST_EMAIL_2 }} | ||
| TEST_PASSWORD_2: ${{ secrets.TEST_PASSWORD_2 }} | ||
| # api-smoke authenticates against Keycloak via ROPC. `dataspace` is a | ||
| # confidential client, so without this the token request returns 401 | ||
| # and the job fails its preflight. | ||
| KEYCLOAK_CLIENT_SECRET: ${{ secrets.KEYCLOAK_CLIENT_SECRET }} | ||
|
|
||
| rollback-on-smoke-failure: |
Comment on lines
+310
to
+336
| name: Finalize Deploy | ||
| needs: [deploy, smoke-tests] | ||
| if: success() | ||
| runs-on: ubuntu-latest | ||
| environment: development | ||
| timeout-minutes: 10 | ||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Ship finalize script to host | ||
| uses: appleboy/scp-action@v0.1.7 | ||
| with: | ||
| host: ${{ vars.EC2_HOST }} | ||
| username: ${{ secrets.EC2_USERNAME }} | ||
| key: ${{ secrets.EC2_PRIVATE_KEY }} | ||
| source: scripts/ci-finalize.sh | ||
| target: ${{ env.DEPLOY_PATH }} | ||
|
|
||
| - name: Prune to current + previous image | ||
| uses: appleboy/ssh-action@v1.0.3 | ||
| with: | ||
| host: ${{ vars.EC2_HOST }} | ||
| username: ${{ secrets.EC2_USERNAME }} | ||
| key: ${{ secrets.EC2_PRIVATE_KEY }} | ||
| script_stop: true | ||
| script: cd "$HOME/${{ env.DEPLOY_PATH }}" && sudo bash scripts/ci-finalize.sh |
Comment on lines
+148
to
+166
| name: Smoke Tests | ||
| needs: deploy-app | ||
| uses: CivicDataLab/CivicDataSpace-test/.github/workflows/run-smoke.yml@CI | ||
| with: | ||
| api_base_url: ${{ vars.DEV_API_BASE_URL }} | ||
| deployed_sha: ${{ github.sha }} | ||
| min_passed: 1 | ||
| secrets: | ||
| HOME_URL_DEV: ${{ secrets.HOME_URL_DEV }} | ||
| TEST_EMAIL_1: ${{ secrets.TEST_EMAIL_1 }} | ||
| TEST_PASSWORD_1: ${{ secrets.TEST_PASSWORD_1 }} | ||
| TEST_EMAIL_2: ${{ secrets.TEST_EMAIL_2 }} | ||
| TEST_PASSWORD_2: ${{ secrets.TEST_PASSWORD_2 }} | ||
| # api-smoke authenticates against Keycloak via ROPC. `dataspace` is a | ||
| # confidential client, so without this the token request returns 401 | ||
| # and the job fails its preflight. | ||
| KEYCLOAK_CLIENT_SECRET: ${{ secrets.KEYCLOAK_CLIENT_SECRET }} | ||
|
|
||
| rollback-on-smoke-failure: |
| } | ||
|
|
||
| return JsonResponse(data) | ||
| return JsonResponse(data, status=200 if overall_status else 503) |
…revert dev was merged into main once before (PR #80, 2026-06-17) and reverted three hours later (PR #82) -- reason given: "The sdk changes are done in wrong query." main took 18 of its own direct commits since (mostly independent rediscoveries of fixes dev already had) while dev moved 70+ commits further ahead, never having gone through the revert itself. ## Conflict resolutions (8 files) Straightforward -- took dev's side wholesale, all fix collateral damage from the blanket revert or superseded by dev's continued work: - .gitignore (main had dropped .DS_Store; dev adds .deploy/.worktrees entries for the CD pipeline work) - Dockerfile (dev fixes the exact things a recent prod survey found still broken live: `--reload` stuck at 1 worker, a 14.1GB image from an accidental CUDA torch install, a missing logs/ dir crash, a stale healthcheck) - dataspace_sdk/__version__.py, pyproject.toml (dev's 0.5.05 is already what's live on PyPI; main's 0.5.02 was stale) - api/views/dynamic_chart_view.py (both sides independently made the same fix; dev's also has incidental Black formatting) - tests/test_settings.py (main's version is literally the pre-PR#80 state the revert rolled back to -- a hardcoded minimal INSTALLED_APPS list the code's own removed comment explicitly warned breaks AUTH_USER_MODEL. dev kept the real fix and added ELASTICSEARCH_DSL_SIGNAL_PROCESSOR since) Needed manual reconciliation, not a clean pick of either side: - api/types/type_resource.py: both sides independently added the same INDEXED_FORMATS/FILE_COUNT_FORMATS entry-count fix. Took dev's file and additionally applied main's PromptFormat(...) wrap on TypePromptResourceDetails.prompt_format -- that field is typed Optional[prompt_format_enum] (a real Strawberry enum) but dev was still passing the raw string. Result is byte-identical to main. - api/utils/data_indexing.py: same entry-count fix on both sides, but dev also still has fetch_resource_data/get_resource_columns/ _build_where_clause/_build_order_by/DataFetchError -- the parameterized SQL data-fetch API that PR #80 introduced and PR #82 reverted. Traced dev's history since the revert: this code hasn't been touched in 3 months, so whatever was "wrong" about it has not been verifiably fixed. Removed it again (along with the now-unused pg_sql import and _FILTER_OPERATORS table), keeping only the entry-count fix both sides agree on. Result is functionally identical to main. ## Deliberately excluded from this merge The fetch_resource_data feature and everything wired to it -- api/views/dataset_data.py, its 3 routes in api/urls.py, and the SDK client's get_resource_data/get_dataset_data/get_prompt_data/ iter_resource_data/_build_data_params methods in dataspace_sdk/resources/datasets.py, plus their tests (tests/test_data_indexing_filters.py, the TestDatasetClientDataFetch class in tests/test_datasets.py). This is the specific thing PR #82's revert named. git's own 3-way merge already excluded most of these automatically (unchanged on dev since the revert + deleted on main = deletion wins, standard merge semantics) -- verified with a full-tree grep that zero references to any of this remain anywhere in the merged result. Still present on dev if this needs picking back up once the underlying query issue is actually understood and fixed. ## Validation done locally (no environment to run the real test suite -- left to CI on the PR) - Full py_compile / compileall sweep: clean, except one pre-existing syntax error (test_grouped_bar_chart.py, a duplicate `options=` kwarg) confirmed identical on both main and dev before this merge -- not touched here, flagged separately. - Full-tree grep confirms zero remaining references to the excluded feature.
saqibmanan
force-pushed
the
merge-dev-into-main
branch
from
September 11, 2026 02:50
0ab318f to
64ee72b
Compare
This was referenced Sep 11, 2026
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
4 participants
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.
Why
mainanddevhave been diverged sincedevwas merged intomainonce (PR #80, 2026-06-17) and reverted 3 hours later (PR #82, reason: "The sdk changes are done in wrong query"). Since thenmaintook 18 of its own commits (mostly independent rediscoveries of fixesdevalready had) whiledevmoved 70+ commits further ahead. This blocks landing the EC2 deploy pipeline onmain(needed forworkflow_dispatch/prod deploys) and is why prod-cds is currently stuck on--reload(1 worker) and a 14.1GB image —dev'sDockerfilealready fixes both,mainnever got them.Conflict resolutions (8 files, went through each individually before merging)
Took
dev's side wholesale (collateral damage from the blanket revert, or superseded bydev's continued work):.gitignore,Dockerfile,dataspace_sdk/__version__.py,pyproject.toml,api/views/dynamic_chart_view.py,tests/test_settings.pyNeeded manual reconciliation:
api/types/type_resource.py— both sides independently added the same entry-count fix; keptdev's file, re-appliedmain's uniquePromptFormat(...)wrap (a real bug — that field is typed as a Strawberry enum,devwas still passing a raw string). Result is byte-identical tomain.api/utils/data_indexing.py— same entry-count fix on both sides, butdevalso still carriesfetch_resource_data/get_resource_columns/_build_where_clauseetc. — the exact feature PR Revert "Dev" #82 reverted. Checkeddev's history since: untouched in 3 months, so whatever was "wrong" about it was never verifiably fixed. Removed it again. Result is functionally identical tomain.Deliberately excluded
The
fetch_resource_datafeature and everything wired to it (api/views/dataset_data.py, its 3 routes, the SDK client'sget_resource_data/get_dataset_data/get_prompt_data/iter_resource_datamethods, and their tests). This is specifically what PR #82's revert named. Verified with a full-tree grep — zero references remain anywhere in the merged result. Still ondevif it's worth picking back up once someone understands what was actually wrong with the query.Validation
py_compile/compileallsweep: clean, except one pre-existing syntax error (test_grouped_bar_chart.py, duplicateoptions=kwarg) confirmed identical on bothmainanddevbefore this merge — unrelated, not touched here..github/workflows/deploy-to-ecs.ymlalso changes (dev's ECS hardening) but its trigger stayspush: devonly, so this merge doesn't newly trigger anything.Test plan
devbeyond the documented exclusion