Skip to content

Add Board Name and BIOS Version to data collection - #1

Open
gdevenyi wants to merge 1 commit into
chenri2006:masterfrom
DouglasNeuroInformatics:feat/board-bios-facts
Open

Add Board Name and BIOS Version to data collection#1
gdevenyi wants to merge 1 commit into
chenri2006:masterfrom
DouglasNeuroInformatics:feat/board-bios-facts

Conversation

@gdevenyi

@gdevenyi gdevenyi commented Aug 5, 2026

Copy link
Copy Markdown

Surfaces two facts that Ansible already gathers but that ansible-cmdb never displayed: ansible_board_name and ansible_bios_version.

Why

  • Board name distinguishes machines that share a product name — common with whitebox/OEM batches where ansible_product_name is identical across visibly different hardware.
  • BIOS version is what you need to track firmware update campaigns across a fleet.

What changed

  • html_fancy: two new overview columns, Board Name and BIOS Version. Both are visible: False, so the default view is unchanged — they're opt-in via the column toggles or -c.
  • html_fancy: matching rows in the hardware detail table, next to Product serial.
  • markdown and markdown_split: matching detail rows.

All use default='', so hosts without the facts render blank rather than erroring.

Testing

Rendered html_fancy, markdown, and markdown_split against the bundled example/ inventory. New fields populate with real values (A06, 5.06, 6.00, VirtualBox) and render empty for hosts lacking the facts.

Note: the vendored lib/yaml* and lib/mako don't import on Python 3.10+ (collections.Hashable was removed), so this was rendered against system PyYAML/Mako. Unrelated to this change.

Note for the maintainer

I've opened a companion PR adding the ansible_cmdline fact. Both insert into the same column list, so whichever merges second will need a trivial one-line rebase. Happy to combine them into one PR if you'd prefer.

Expose the ansible_board_name and ansible_bios_version facts, which are
gathered by Ansible but were not surfaced anywhere in the output.

- html_fancy: add optional (hidden by default) "Board Name" and
  "BIOS Version" overview columns, and rows in the hardware detail table.
- markdown / markdown_split: add the matching detail rows.

Both fields are useful for hardware inventory: board name distinguishes
machines that share a product name, and BIOS version is needed to track
firmware update campaigns.
Copilot AI lite review requested due to automatic review settings August 5, 2026 18:33

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR extends ansible-cmdb’s output templates to display two additional Ansible facts—ansible_board_name and ansible_bios_version—so users can distinguish similar product-name hardware and track firmware versions across fleets.

Changes:

  • Add “Board Name” and “BIOS Version” as optional (hidden-by-default) overview columns in the html_fancy template.
  • Add matching “Board name” and “BIOS version” rows to the hardware detail sections in html_fancy, markdown, and markdown_split.
  • Use safe defaults (default='' / .get(..., '')) so missing facts render blank instead of failing.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/ansiblecmdb/data/tpl/markdown.tpl Adds Board name and BIOS version lines to the Hardware section using .get(..., '').
src/ansiblecmdb/data/tpl/markdown_split_detail.tpl Adds Board name and BIOS version lines to the Hardware section in split-detail Markdown output.
src/ansiblecmdb/data/tpl/html_fancy_defs.html Adds two hidden-by-default overview columns and corresponding col_* accessors; adds two rows in the hardware detail table.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@gdevenyi

gdevenyi commented Aug 5, 2026

Copy link
Copy Markdown
Author

Merge order for the 11 open PRs

I merged all eleven locally to check they compose. Only two conflicts arise, and both have a resolution that is not the obvious one — details below so you don't have to rediscover them.

Suggested order

# PR Result
1 #4 wrapper interpreter selection clean
2 #5 pyproject / drop distutils clean
3 #11 txt_table Kernel column clean
4 #1 Board Name + BIOS Version clean
5 #2 kernel Cmdline conflict A
6 #7 markdown_split iface dicts clean
7 #8 deterministic output clean
8 #3 ini :vars parsing clean
9 #6 YAML inventories clean
10 #9 --limit separator clean
11 #10 executable-but-unrunnable inventory conflict B

The two conflicts are inherent pairwise (#1#2 and #6#10 touch the same lines), so no ordering avoids them — but nothing else collides, and reordering within the clean groups is safe.


Conflict A — #1 vs #2, html_fancy_defs.html

Both append a column. Two hunks: the cols list and the <%def> block. Both sides are additive, so "keep both" is right — but concatenating the two sides naively produces a silently broken template.

The conflict region ends before a shared </%def>, so col_biosversion loses its closing tag and swallows col_cmdline. Mako still parses it; the render just fails at runtime.

After resolving, col_biosversion must read:

<%def name="col_biosversion(host, **kwargs)">
  ${jsonxs(host, 'ansible_facts.ansible_bios_version', default='')}
</%def>

<%def name="col_cmdline(host, **kwargs)">

Quick check — these must be equal:

grep -c '<%def ' src/ansiblecmdb/data/tpl/html_fancy_defs.html
grep -c '</%def>' src/ansiblecmdb/data/tpl/html_fancy_defs.html

The cols list hunk needs no care: keep all three entries.

Conflict B — #6 vs #10, ansible.py and test.py

test.py is trivial (both add imports and test methods; keep both sides).

ansible.py is not. Both restructure the same dispatch chain — #6 inserts a YAML branch, #10 makes the executable branch fall back to static parsing. Taking either side loses the other, and combining them naively sends a chmod +x'ed .yml to the ini parser, which is exactly the bug #6 exists to fix.

What worked was factoring the shared path out:

def _parse_static_inventory(self, inventory_path):
    """
    Read a non-executable inventory file. YAML inventories can't be read
    by the ini parser, so they are handed to ansible-inventory instead.
    """
    if inventory_path.lower().endswith((".yml", ".yaml")):
        self._parse_ansible_inventory(inventory_path)
    else:
        self._parse_hosts_inventory(inventory_path)

called from both places:

if os.path.isfile(p) and util.is_executable(p):
    if not self._parse_dyn_inventory(p):
        self._parse_static_inventory(p)      # exec fallback
elif os.path.isfile(p):
    self._parse_static_inventory(p)          # ordinary file

That handles a case neither PR does alone: an executable YAML inventory now reaches ansible-inventory instead of the ini parser.


Verified end state

With all eleven merged: 21/21 tests pass, all seven single-file templates render and are byte-stable across repeated runs, markdown_split writes 22/22 host pages and html_fancy_split 23 files.

I'm happy to push a single pre-merged branch if you'd rather take it in one go than resolve these yourself.

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.

2 participants