Repository navigation
Conversation
…st endpoints exabyte-api-client's EntityEndpoint.list(query, projection) wraps filters as query=<json>&projection=<json> blob params. Several web-app REST list endpoints were migrated to use cases validated by a JSON schema with additionalProperties: false, which silently strips any undeclared param - so every one of these blob queries had become a silent no-op, returning unfiltered (or empty) results. Switches the shared material/property/job API helpers and the notebooks that called .list() directly to .request() with the flat params each use case actually declares. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…dd missing pandas dep
Notebooks used client.projects.list()/client.jobs.create_set()/client.properties.get_for_job(),
which build their filters as raw dot-notation query blobs or the old {name, owner} shape.
Endpoints migrated to validated use cases silently drop undeclared keys (AJV removeAdditional)
or reject the request outright, so these calls returned unfiltered/wrong results (e.g. always
the account's first project or property) or a 422. Added get_default_project (mirroring the
already-fixed get_or_create_material/get_or_create_workflow pattern) and applied it across the
affected notebooks; fixed jobs.create_set's param shape and get_or_create_workflow's own
.list() call in equation_of_state.ipynb and workflow/api.py; swapped
equation_of_state.ipynb's direct client.properties.get_for_job() (broken in the vendored
api-client itself) for the already-fixed get_properties_for_job() helper.
Also added the pandas dependency the mat3ra-standata package needs, missing from
specific_examples/api_examples' package lists.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…rver round-trip create_job(workflow=...) was passing saved_workflow_response (the server's saved copy) instead of the notebook's own locally-built workflow object. The server round-trip drops context providers' defaults (e.g. the gamma-point kgrid), so job creation's unconditional renderAsync() recomputes them from the material instead - for neb.ipynb this silently swapped a gamma-point NEB calculation for a material-derived k-grid, converging to a qualitatively different (and wrong-signed) reaction profile. create_job() already strips _id from the workflow it's given, so passing the local object works exactly like main did before saved_workflow_response was substituted in as an incidental part of an unrelated query-blob fix. Also fixes stray \uXXXX-escaped unicode (from an earlier json.dump without ensure_ascii=False) in homo_lumo_frequency.ipynb. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…sites The 3 fix commits on this branch broke the repo's black formatting (long .request(...) calls not wrapped) - CI's run-linter failed, which also skipped run-py-tests and publish-py-package downstream. No behavior change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The 3 fix commits on this branch moved several material/property functions
from client.materials.list()/client.properties.list() (raw query=/projection=
blobs, silently dropped by the migrated list endpoints) to
client.materials.request()/client.properties.request() with flat params -
but left these two test files mocking/asserting the old .list() call shape,
so every test touching a migrated function failed.
- Retarget all mocks/assertions from `.list(...)` to
`.request("GET", <endpoint>.name, params=..., headers=...)`, translating
each query's dot-notation/operator shape to the flat params the production
code now actually sends (e.g. "owner._id" -> "ownerId", "hash" ->
"hashes", "$regex" name matching -> no server-side equivalent, filtered in
Python).
- find_relaxed_material's job-matching narrowing ("_material._id": {"$in":
[...]}) has no flat equivalent either and is now done client-side, so
FINISHED_JOB (and the two job fixtures in its "skips a final structure
with the same hash" test) needed a "_material" field to match against -
they had none, so every finished-job branch was silently unreachable.
- _get_properties_for_job now unwraps holder["data"] (matching
get_for_job()'s old return shape), so test_get_final_structure_for_job's
fixture needed the same {"data": {...}} wrapping its mock is standing in
for.
Verified locally: 72 passed (2 pre-existing, unrelated asyncio-marker
failures in test_jupyterlite_interrupts.py excluded - confirmed present on
origin/main too, needs pytest-asyncio which isn't a project dependency).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…material JobsList already declares and implements a flat "materialId" param (JobDAO#filterByMaterialId, $in-matching _material._id/_materials._id server-side) - the earlier fix's comment claiming no such filter exists was wrong. Narrowing by material id server-side means we no longer have to fetch every one of the account's finished jobs and filter in Python, and skip the jobs call entirely when no materials match by hash. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
| "# Using .request() with flat params instead of .list(): .list() always wraps its argument as a\n", | ||
| "# query=<json> blob, which the materials/workflows list endpoints (migrated to validated use\n", | ||
| "# cases) silently drop since they only accept flat, declared keys.\n", | ||
| "default_material = client.materials.request(\n", |
Member
There was a problem hiding this comment.
list is an easier to understand concept
Maybe this problem should be handled on the web application side, not in python codebase?
…ow_default
Replaces the `.request("GET", ..., {"isDefault": "true", "ownerId": ...})`
workaround with the API client's `show_default(account_id)`, so notebooks no
longer need to know about the flat-param list queries.
- `get_default_material` / `get_default_workflow` live in their own small
`default.py` modules next to `get_default_project`. They are kept apart from
`api.py` because notebooks that install only the `api` package group cannot
import `mat3ra.made` / `mat3ra.wode`; a test guards that they import only
the API client.
- `create_and_submit_job` uses the two new helpers and no longer re-fetches
the workflow it already has.
- `get-file-from-job` and the materials_designer workflow notebooks that
inlined the projects lookup now use `get_default_project`.
Needs a `mat3ra-api-client` release that includes `show_default`
(mat3ra/api-client feature/SOF-8032).
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…request() workarounds
The list endpoints filter on the flat parameters of their use cases and drop
the Mongo query `list()` used to send. Instead of calling `.request()` with flat
params, notebooks and helpers use `list()` with those parameters directly, e.g.
`client.materials.list({"name": name, "ownerId": ACCOUNT_ID})`.
- Helpers (`material`, `workflow`, `job`, `property` api): lookups by hash, name,
id, set and job use `ownerId`, `hashes`, `setId`, `materialId`, `status`, ...
Lookups that must find entities inside sets pass `globalSearch`. An empty id
list returns early in `find_relaxed_material` (an empty filter matches all).
`get_properties_for_job` is back to `get_for_job`, which filters on the unit.
- Notebooks: the project, material and workflow default lookups use
`get_default_*`; elemental references use `tags` + `metadataElement`;
`get_materials_by_formula` filters on `formula`; `get_workflows` searches with
`quickSearch`; `analyze_convex_hull` uses `formula`, `groupPrefix` and
`precisionValue`. Name lookups match the exact name.
- Their "Set Parameters" cells describe the list filters instead of a Mongo
selector.
- `formation_energy` keeps the Mongo-style query of its standata template, which
the client translates.
- Tests follow the new call shapes.
Needs the web-app list filters (`formula`, `unitId`, `precisionValue`) deployed
and a `mat3ra-api-client` release with `list()` parameters and `show_default`
(mat3ra/api-client feature/SOF-8032).
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…nd helpers The api-client list() now sends Mongo-style queries as advancedSearches, so the notebooks keep their original form: no show_default lookups, no flat list parameters, no get_default_* helpers. What stays from the PR: pandas in config.yml, the TEXT_SHADOW removal in plot.py, and the notebook fixes that do not depend on list() (embedding the full workflow when creating a job, the saved-workflow response variables, and the project id in the equation_of_state set). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
One conflict, in defect_formation_energy.ipynb: main reworked the notebook (SOF-7975), so it takes main's version. That also drops the leftover get_or_create_workflow call whose response was no longer used since the job is built from the local workflow. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
create_job was given the server's saved copy of the workflow, which drops the context providers' defaults, so job creation recomputes them from the material. Same fix as 1692a3f for the other notebooks; the notebook is back to main's version. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Member
Author
|
The only relevant changes are now in mat3ra/api-client#48 |
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
2 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.
…st endpoints
exabyte-api-client's EntityEndpoint.list(query, projection) wraps filters as query=&projection= blob params. Several web-app REST list endpoints were migrated to use cases validated by a JSON schema with additionalProperties: false, which silently strips any undeclared param - so every one of these blob queries had become a silent no-op, returning unfiltered (or empty) results. Switches the shared material/property/job API helpers and the notebooks that called .list() directly to .request() with the flat params each use case actually declares.