Skip to content

fix(data-importer): make CSV imports work, and consolidate the open Dependabot updates - #202

Open
petercrocker wants to merge 5 commits into
mainfrom
pc-20260814-fix-csv-importer
Open

fix(data-importer): make CSV imports work, and consolidate the open Dependabot updates#202
petercrocker wants to merge 5 commits into
mainfrom
pc-20260814-fix-csv-importer

Conversation

@petercrocker

@petercrocker petercrocker commented Aug 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Fixes the CSV Data Importer, which a customer reported as "doesn't work", and folds in the five open Dependabot PRs (#197, #198, #199, #200, #201) as requested.

Note on scope: bundling dependency bumps with a functional fix makes review and bisecting harder, and hand-merging the Dependabot changes means those PRs won't auto-close. This was an explicit request; the work is split into two commits so each half can be read on its own.

The reported bug

Importing a CSV of racks produced a blank page: no data, no error, nothing. Reproduced against a fresh Infrahub 1.10.7 + Emma stack using the customer's own schema and files. Five defects were involved.

1. Relationship references couldn't be a plain human-friendly ID. parse_item() treated the first __-separated segment of every value as a kind name, so rtp1 in a site column became a lookup for a kind called rtp1:

Emma version site=rtp1
0.3.0 (the published :latest) silently became []; site was dropped from the mutation, so Infrahub rejected each row with site is mandatory for LabRack
0.6.2 (main) unhandled SchemaNotFoundError: Unable to find the schema 'rtp1' traceback

Only the undocumented LabSite__rtp1 form worked. The peer kind is already known from the relationship schema, so it's now used to resolve a bare value. Kind__identifier is still honoured and is still required for relationships pointing at a generic, where the concrete kind can't be inferred. This is what docs/features/data-import-export.mdx has always promised.

2. Failed imports reported success. execute_batch() displayed each rejected row but returned None, and the caller only counted errors if execute_batch itself raised — which it never does, because the batch is created with return_exceptions=True. Every row could fail and the toast still said "Loading completed with success".

3. An unrecognised column aborted the whole import. Warnings and errors shared one list, and any entry in it skipped past the preview. So one stray column blocked an otherwise valid file.

4. Whitespace in headers was significant and invisible. The customer's file had a height_u header. It failed to match the schema, and the message rendered the name unquoted — so the trailing space was undetectable, and the message was indistinguishable from a genuinely misnamed column.

5. Messages were toast-only. They faded after ~4s, leaving a blank page. This is why the symptom was "nothing happens" rather than an error.

Two of the four rows in the customer's sites.csv are genuinely bad data (RTP-BIG-SITE violates the name regex; europe isn't a valid region choice). Infrahub correctly rejects those — that part was never a bug, but #2 hid it.

What changed

  • Validation and preprocessing move to emma/csv_import.py. The page executes Streamlit code at import time, so nothing in it could be unit tested; the relationship lookup is injected as a callable for the same reason. pages/data_importer.py is now UI only.
  • execute_batch() returns a failure count; the page reports "N of M row(s) imported".
  • Message severity is honoured — warnings inform, errors block.
  • Headers and values are stripped; messages quote names so whitespace is visible.
  • Messages render inline and name the CSV line they came from.
  • Docs rewritten to describe the reference formats that actually work.
  • 41 new tests; 62 pass on Python 3.10 and 3.12.

Incidental fixes found on the way: is_uuid() raised AttributeError on non-string input (a numeric CSV cell); literal_eval() raised an unhandled SyntaxError on a bracketed non-literal; and get_cached_schema() and friends were annotated as returning the schema definition models while returning the API models — which made the Schema Visualizer filter on classes that never match, so both its lists were always empty. That page is currently unreachable (nav entry commented out in menu.py:36), so it's latent, which is why it has no changelog entry.

Verified against a live instance

Rebuilt the image from this branch against Infrahub 1.10.7 and re-ran the customer's files:

  • racks.csv (trailing-space header + bare rtp1) → all 3 racks created, height_u picked up, each linked to the right site
  • sites.csv → Loading completed with 2 error(s): 2 of 4 row(s) imported.
  • unresolvable reference → Line 3: could not resolve 'nope-site' in column 'site': no LabSite found with human-friendly ID ['nope-site']. — inline, no traceback
  • unmappable column → warns, and the valid row still imports

Dependency updates

Three of the five PRs touch the same two lockfiles and can't be merged independently, so the Python lock was regenerated from the updated constraints and checked against every version the security PRs ask for. All met or exceeded. Two are resolved by removal rather than a bump: streamlit 1.61 replaced tornado with starlette, and node-forge is no longer reachable from the docs tree.

Four things had to be resolved:

  • Bump the all group with 12 updates #198 is unresolvable as written. It raises ruff to >=0.16.1, but infrahub-sdk[all] → ariadne-codegen pins ruff<0.16. Capped at <0.16 with a comment to lift it later.
  • Bump the npm_and_yarn group across 1 directory with 18 updates #201 breaks the docs build — that's why its checks are red. webpack 5.109 tightened the ProgressPlugin options schema; webpackbar 6.0.1 still passes name, color, reporters. Overridden to webpackbar ^7.0.0, which matches the new schema; docs build confirmed passing.
  • pytz was never declared despite being imported by emma/git_utils.py and pages/schema_library.py. It arrived via pandas, and streamlit >=1.60 pulls pandas 3, which dropped it — so both modules would fail to import on Python 3.12. Now explicit.
  • uv.lock had drifted from pyproject.toml. uv lock --check fails on main. Dependabot's pip ecosystem updates pyproject.toml without regenerating the lock, and CI's files-changed filter doesn't treat either file as Python — so python-lint and pytest skip on those PRs and the drift accumulated unnoticed. That's also how Bump the all group with 12 updates #198's unresolvable constraint sat there looking green. Worth fixing in dependabot.yml/CI separately.

CI status

All checks green: ruff check, ruff format, mypy, pylint (10.00/10), pytest (69 tests on 3.10 and 3.12), streamlit-test, markdown-lint, yaml-lint, and the docs build.

Getting there meant clearing lint and type debt that was already red on main, because the stale lockfile hid it: CI's uv sync re-resolves to a newer ruff than the lockfile pins, so ruff check . reported 87 findings and mypy 27 on main today. Each mypy error turned out to be a real defect rather than a bad annotation:

  • convert_node_to_dict() dereferenced a raise_when_missing=False store lookup, so a peer missing from the store crashed the Data Exporter instead of falling back.
  • load_schema() / check_schema() declared schemas optional with a None default the SDK cannot accept; all four call sites always pass it.
  • get_client_async() forwarded address=None to InfrahubClient, which takes a str.
  • template_builder's TemplateSyntaxError branch called identify_faulty_jinja_code(e) — that helper takes a rich Traceback and returns a list of (Frame, Syntax) pairs, which the next line concatenated onto a string. That branch could only ever raise.

The three reachable pages whose code paths changed were re-verified against the live stack: Data Exporter renders every rack with its site resolved to LabSite__rtp1, Schema Loader checks and loads a schema, Data Importer imports. template_builder needs OpenAI credentials and wasn't exercised, but its previous behaviour was an unconditional exception.

Also worth knowing

The published registry.opsmill.io/opsmill/emma:latest is v0.3.0, built 2025-01-28, while main is v0.6.2. The documented quickstart (curl https://infrahub.opsmill.io/latest-emma | docker compose -f - up -d) therefore ships an 18-month-old Emma, so none of this reaches users until that image is rebuilt.

@petercrocker
fix(data-importer): make CSV imports resolve relationships and report…
2ff8caa 
… failures

A customer reported that importing a CSV of racks did nothing: no data, no
error, a blank page. Four separate defects combined to produce that, and a
fifth made a wholly failed import look successful.

Relationship references could not be given as a plain human-friendly ID.
parse_item() read the first `__`-separated segment of every value as a kind
name, so `rtp1` in a `site` column became a lookup for a kind called `rtp1`.
On the published 0.3.0 image that silently yielded an empty list and the
relationship was dropped from the mutation, leaving Infrahub to reject the row
as missing a mandatory field; on current main it raises SchemaNotFoundError as
an unhandled traceback. The peer kind is already known from the relationship
schema, so it is now used to resolve a bare value, and the `Kind__identifier`
form is kept for relationships that point at a generic, where the concrete
kind cannot be inferred. This is the behaviour the docs have always described.

Failed imports reported success. execute_batch() displayed each rejected row
but returned None, and the caller only counted errors if execute_batch itself
raised, which it never does because the batch is created with
return_exceptions=True. It now returns the failure count and the page reports
how many of the file's rows were imported.

A column matching nothing in the schema aborted the whole import. Warnings
were collected in the same list as errors and any entry in that list skipped
straight past the preview, so an unrecognised column blocked an otherwise
valid file. Severity is now honoured: warnings are shown and the import
proceeds.

Whitespace in headers and values was significant. A header of `height_u `
failed to match the schema, and because the message rendered the name
unquoted the trailing space was invisible, making it indistinguishable from a
misnamed column. Both are now stripped, and messages quote the name.

Validation messages were only shown as toasts, which faded after a few
seconds and left the page blank with no indication of the problem. They are
now rendered inline and name the CSV line they came from.

The logic moves to emma/csv_import.py so it can be unit tested; the page
executes Streamlit code at import time and cannot be imported by a test. The
relationship lookup is injected as a callable for the same reason.

Also fixed along the way:

- is_uuid() raised AttributeError on non-string input, which a numeric CSV
  cell produces.
- literal_eval() on a bracketed value that is not a list literal raised an
  unhandled SyntaxError.
- get_cached_schema() and friends were annotated as returning the schema
  definition models while returning the API models. The Schema Visualizer
  filtered on the definition models as a result, so both of its lists were
  always empty; that page is currently unreachable (its nav entry is commented
  out in menu.py), so this is latent rather than user-visible, and it is why
  there is no changelog entry for it.

Verified against a live Infrahub 1.10.7 with the customer's own files: the
racks CSV now imports all three rows with their sites linked, and the sites
CSV reports "2 of 4 row(s) imported" instead of success.
@petercrocker
chore(deps): consolidate the open Dependabot updates
5bfb357 
Applies the five open Dependabot PRs as one change, since three of them touch
the same two lockfiles and cannot be merged independently:

- #197 github-actions: setup-python v6 -> v7, setup-node v6 -> v7
- #198 pip: streamlit, infrahub-sdk, langchain, openai, langchain-openai,
  gitpython, mypy, pylint, pytest, types-pyyaml, types-pytz
- #199 uv (security): langchain, aiohttp, dulwich, pillow, pydantic-settings,
  python-dotenv, ujson, urllib3
- #200 uv (security): tornado
- #201 npm_and_yarn (security), docs/: 16 packages including webpack,
  webpack-dev-server, postcss, lodash, nanoid, ws

#199 and #200 only changed uv.lock, so rather than reconciling three
conflicting lockfiles the Python lock was regenerated from the updated
constraints and checked against every version those PRs asked for. All are met
or exceeded. Two are resolved by removal rather than by a bump: streamlit 1.61
replaced tornado with starlette, and node-forge is no longer reachable from the
docs tree.

Four things had to be resolved to make these land together:

- #198 as written is unresolvable. It raises ruff to >=0.16.1, but
  infrahub-sdk[all] depends on ariadne-codegen, which pins ruff <0.16. ruff is
  capped at <0.16 with a comment, to be lifted when ariadne-codegen allows it.

- #201 as written breaks the docs build, which is why its checks are red.
  webpack 5.109 tightened the ProgressPlugin options schema, and webpackbar
  6.0.1 still passes `name`, `color` and `reporters`, so Docusaurus fails with
  a ValidationError. webpackbar is overridden to ^7.0.0, which matches the new
  schema. The docs build was confirmed to succeed with it.

- pytz is imported directly by emma/git_utils.py and pages/schema_library.py
  but was never declared; it arrived transitively through pandas. streamlit
  >=1.60 pulls pandas 3, which dropped pytz, so both modules would fail to
  import on Python 3.12. It is now an explicit dependency.

- uv.lock had drifted out of sync with pyproject.toml, because Dependabot's pip
  ecosystem updates pyproject without regenerating the lock and CI's
  files-changed filter does not treat either file as Python, so the python-lint
  and pytest jobs skip on those PRs. `uv lock --check` fails on main. CI's
  `uv sync` therefore re-resolves and picks up a much newer ruff than the
  lockfile pins, and `ruff check .` reports 87 findings on main today. Those
  are fixed here: 84 are ruff's own config lint asking for rule names instead
  of codes in pyproject.toml, and the rest are an empty `if` and two oversized
  try clauses.

ruff, ruff format, pylint (10.00/10) and pytest are green on Python 3.10 and
3.12. mypy still reports 18 pre-existing errors, down from 27 on main; they sit
in convert_node_to_dict, convert_schema_to_dict, get_client_async and the
schema load helpers, are unrelated to this work, and need their own pass.
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Aug 14, 2026 •
edited
Loading

Copy link
Copy Markdown

Deploying emma with  Cloudflare Pages  Cloudflare Pages

Latest commit: b829b8c
Status: ✅  Deploy successful!
Preview URL: https://90954b15.emma-anv.pages.dev
Branch Preview URL: https://pc-20260814-fix-csv-importer.emma-anv.pages.dev

View logs

@petercrocker
fix(changelog): exclude towncrier fragments from markdownlint
ae77447 
Fragments are prose snippets spliced into CHANGELOG.md, so MD041's
first-line-heading rule does not apply to them: a heading would be rendered
into the changelog body.
@petercrocker
fix: clear the remaining mypy errors
c41ff6c 
python-lint was failing on 18 mypy errors that predate this branch. Each turned
out to be a real defect rather than a bad annotation.

emma/infrahub.py:

- convert_node_to_dict() dereferenced the result of a store lookup made with
  raise_when_missing=False, so a peer that was not in the store crashed the Data
  Exporter instead of falling back. The lookup and the description of a peer are
  now separate helpers, and a miss falls back to the id we already hold.
- its `peers` list was annotated list[dict[str, Any]] while only ever holding
  the strings that describe each peer.
- `data` in convert_schema_to_dict() had no annotation, so mypy inferred its
  value type from the literal and rejected .append() on the two list entries.
- get_client_async() forwarded address=None to InfrahubClient, which is typed to
  take a str and documents "" as "resolve it yourself". Both spellings behave
  identically, verified against the SDK, so it now passes "".
- load_schema() and check_schema() declared `schemas` optional and defaulted it
  to None, which the SDK cannot accept. All four call sites always pass it, so it
  is now required.
- get_objects_as_df() declared two bools as `bool | None` with True defaults.

pages/template_builder.py: the TemplateSyntaxError branch called
identify_faulty_jinja_code(e), but that SDK helper takes a rich Traceback and
returns a list of (Frame, Syntax) pairs, which the next line concatenated onto a
string. The branch could only ever raise. A syntax error is raised while parsing,
so there are no rendered frames to inspect and the exception already carries the
line number, which is what is now reported.

Verified against the live stack that the three reachable pages whose code paths
changed still work: Data Exporter renders every rack with its site resolved,
Schema Loader checks and loads a schema, Data Importer imports. template_builder
needs OpenAI credentials and was not exercised, but its previous behaviour was
an unconditional exception.

ruff, ruff format, mypy, pylint (10.00/10) and 69 tests are green on 3.10 and 3.12.
@petercrocker
chore(release): prepare 0.7.0
b829b8c 
Minor rather than patch: the CSV importer accepts reference formats it did not
before, and the dependency refresh moves streamlit 1.56 -> 1.61, which brings
pandas 3 and drops tornado.

Note that the previous tag is v0.6.1 while pyproject.toml already said 0.6.2, so
0.6.2 was never released; this skips it. towncrier had not been run since 0.2.0
either, so the changelog jumps from 0.2.0 straight to 0.7.0.

One manual touch-up: towncrier leaves two blank lines before the preceding
release heading, which trips markdownlint's MD012 and does not match the rest of
the file. Collapsed to one.
@petercrocker
petercrocker marked this pull request as ready for review August 14, 2026 16:37
@petercrocker
petercrocker requested a review from a team as a code owner August 14, 2026 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@petercrocker