Add asset restore/audit/licenses/files/labels APIs and tests#8
Conversation
Enhance integration testing setup by adding pytest fixtures in conftest.py for shared test data (e.g., manufacturers, categories) and utilities like unique naming and ID extraction to prevent collisions. Introduce new test files for custom fields and fieldsets, verifying create, read, update, delete operations via the SnipeIT client. Update pyproject.toml to include dev dependency group for testing tools like pytest, coverage, and ruff.
Add *.html and *.tmp patterns to prevent committing generated HTML outputs and temporary files, keeping the repository clean.
- Updated `Asset.audit()` docstring to clarify ID-based auditing via POST /hardware/{id}/audit, with optional fields like location_id and next_audit_date.
- Added `Asset.restore()` method to handle soft-deleted assets via POST /hardware/{id}/restore.
- In `AssetsManager`:
- Added `audit_by_id()` for targeted audits (POST /hardware/audit/{id}).
- Added `list_audit_overdue()` and `list_audit_due()` for retrieving due/overdue audits (GET endpoints).
- Refactored `get_by_serial()` to robustly handle single-object or envelope responses (with rows/total), improving error handling for not-found or multi-match cases.
- Retained `create_maintenance()` as-is per request, for future handling.
These changes enhance asset management capabilities and API response flexibility in the Snipe-IT integration.
WalkthroughAdds ignore patterns in .gitignore and restructures pyproject.toml with dependency groups. Expands assets API: restore, audits, licenses, files, labels handling, and robust serial lookup. Introduces extensive integration scaffolding and CRUD tests across resources; replaces two legacy asset integration tests. Adds unit tests for new asset endpoints and flows. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Caller
participant AM as AssetsManager
participant API as Snipe-IT API
participant FS as Filesystem
rect rgba(200,220,255,0.3)
note over Dev,AM: Labels generation (PDF or JSON/base64)
Dev->>AM: labels(save_path, assets_or_tags)
AM->>API: POST /hardware/labels (accept pdf,json)
alt PDF response
API-->>AM: application/pdf (binary)
AM->>FS: write PDF bytes
AM-->>Dev: return save_path
else JSON response with file contents
API-->>AM: application/json { pdf_base64 | payload.file_contents }
AM->>AM: decode base64
AM->>FS: write PDF bytes
AM-->>Dev: return save_path
else Error JSON
API-->>AM: error payload
AM-->>Dev: raise error
end
end
rect rgba(200,255,200,0.3)
note over Dev,AM: File operations
Dev->>AM: upload_files(asset_id, paths, notes?)
AM->>API: POST /hardware/{id}/files (multipart)
API-->>AM: upload result
AM-->>Dev: result
Dev->>AM: list_files(asset_id)
AM->>API: GET /hardware/{id}/files
API-->>AM: files[]
AM-->>Dev: files[]
Dev->>AM: download_file(asset_id, file_id, save_path)
AM->>API: GET /hardware/{id}/files/{fid}/download
API-->>AM: file bytes
AM->>FS: write bytes
AM-->>Dev: return save_path
Dev->>AM: delete_file(asset_id, file_id)
AM->>API: DELETE /hardware/{id}/files/{fid}
API-->>AM: 204/ok
AM-->>Dev: None
end
rect rgba(255,240,200,0.35)
note over Dev,AM: Restore and audits
Dev->>AM: restore(asset_id) or asset.restore()
AM->>API: POST /hardware/{id}/restore
API-->>AM: ok
AM->>API: GET /hardware/{id}
API-->>AM: asset
AM-->>Dev: asset
Dev->>AM: audit_by_id(asset_id, payload)
AM->>API: POST /hardware/audit/{id}
API-->>AM: audit result
AM-->>Dev: result
Dev->>AM: list_audit_due()/overdue()
AM->>API: GET /hardware/audit/(due|overdue)
API-->>AM: {}
AM-->>Dev: {}
end
rect rgba(255,220,230,0.35)
note over Dev,AM: Robust serial lookup
Dev->>AM: get_by_serial(serial)
AM->>API: GET /hardware?search=serial:...
alt Single object
API-->>AM: {id, ...}
else Envelope
API-->>AM: {rows:[{id,...}], total:n}
AM->>AM: validate 1 match
end
AM-->>Dev: Asset or not found/multi-match error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/integration/conftest.py (1)
27-45: Don’t override env; prefer env if set, fallback to docker/api_key.txt.Current logic always sets URL and requires the file token, which breaks CI where env is already provided.
- # URL for local Snipe-IT in docker-compose - os.environ["SNIPEIT_TEST_URL"] = "http://localhost:8000" - - # Ensure API key exists and is non-empty; otherwise skip integration suite - if not api_key_file.exists(): - pytest.skip( - "Integration tests require docker/api_key.txt. " - "Run 'make test-integration' to start the local Snipe-IT and generate a token." - ) - - token = api_key_file.read_text().strip() - if not token: - pytest.skip( - "Integration tests require a non-empty docker/api_key.txt. " - "Run 'make test-integration' to start the local Snipe-IT and generate a token." - ) - - os.environ["SNIPEIT_TEST_TOKEN"] = token + # Use URL from env if present; otherwise default to local docker-compose + os.environ.setdefault("SNIPEIT_TEST_URL", "http://localhost:8000") + + # Prefer SNIPEIT_TEST_TOKEN from env; otherwise read docker/api_key.txt; skip only if neither. + token = os.environ.get("SNIPEIT_TEST_TOKEN") + if not token: + if not api_key_file.exists(): + pytest.skip( + "Integration tests require SNIPEIT_TEST_TOKEN env var or docker/api_key.txt. " + "Run 'make test-integration' to start the local Snipe-IT and generate a token." + ) + token = api_key_file.read_text().strip() + if not token: + pytest.skip( + "Integration tests require non-empty SNIPEIT_TEST_TOKEN env var or docker/api_key.txt." + ) + os.environ["SNIPEIT_TEST_TOKEN"] = token
🧹 Nitpick comments (36)
.gitignore (1)
68-70: Don't blanket-ignore lockfiles.Ignoring
*.lockcan hinder reproducible builds (Poetry, Pipenv, uv). Prefer committing lockfiles and scoping ignores narrowly if needed.Apply this diff to keep lockfiles versioned while still ignoring temporary artifacts:
-*.lock *.html *.tmp +# If you truly need to ignore specific editor/system locks, scope narrowly: +# .*.swp handled above; add others explicitly rather than *.lockpyproject.toml (2)
31-33: Use setuptools package discovery instead of a hard-coded list.Hard-coding packages risks missing new subpackages. Let setuptools find
snipeit*.-[tool.setuptools] -packages = ["snipeit", "snipeit.resources"] +[tool.setuptools.packages.find] +include = ["snipeit*"]
19-29: Avoid duplicating dev dependencies in two mechanisms.You already declare
devunder[project.optional-dependencies]. Having a seconddevunder[dependency-groups]risks drift. Pick one (extras for pip, or groups for uv) and document how to install.Minimal cleanup to rely on standard extras:
-[dependency-groups] -dev = ["requests-mock", - "pytest", - "pytest-cov", - "coverage", - "hypothesis", - "mutmut<3", - "ruff", - "pyright", -]Optionally add a note to README:
pip install -e .[dev].Also applies to: 34-43
tests/integration/resources/test_licenses.py (1)
34-37: Narrow cleanup exceptions; don't swallow real errors.Catching
Exceptionandpasscan hide API/transport issues. Ignore only “not found”, surface others.- try: - c.licenses.delete(id_int(lic)) - except Exception: - pass + try: + c.licenses.delete(id_int(lic)) + except SnipeITApiError as e: # type: ignore[name-defined] + if getattr(e, "status_code", None) not in (404,): + raiseAdd at top if not already imported:
-from snipeit import SnipeIT +from snipeit import SnipeIT +from snipeit.exceptions import SnipeITApiErrortests/integration/resources/test_fieldsets.py (2)
29-33: Make the delete assertion resilient.Asserting on error text
"in use"is brittle. Prefer checking status code (e.g., 422/400) when deletion is blocked.- except SnipeITApiError as e: - assert "in use" in str(e).lower() + except SnipeITApiError as e: + assert getattr(e, "status_code", None) in (400, 409, 422)
35-38: Don't blanket-suppress cleanup failures.Limit to not-found; re-raise unexpected errors.
- try: - c.fieldsets.delete(id_int(fs)) - except Exception: - pass + try: + c.fieldsets.delete(id_int(fs)) + except SnipeITApiError as e: + if getattr(e, "status_code", None) not in (404,): + raisetests/integration/resources/test_manufacturers.py (2)
34-35: Verify thatsave()persisted the change.After
updated.save(), fetch and assert the note to validate instance persistence.updated.notes = f"note-{run_id}" updated.save() + got_after = c.manufacturers.get(id_int(updated)) + assert getattr(got_after, "notes", "") == f"note-{run_id}"
37-40: Tighten cleanup exception handling.Only ignore 404; re-raise others so test failures aren’t masked.
- try: - c.manufacturers.delete(id_int(created)) - except Exception: - pass + try: + c.manufacturers.delete(id_int(created)) + except SnipeITApiError as e: + if getattr(e, "status_code", None) not in (404,): + raiseAdd import if missing:
from snipeit.exceptions import ( SnipeITNotFoundError, SnipeITApiError, )tests/integration/resources/test_fields.py (1)
35-38: Narrow exception in cleanup.Mirror other tests: ignore only 404 to avoid hiding real API issues.
- try: - c.fields.delete(id_int(fld)) - except Exception: - pass + try: + c.fields.delete(id_int(fld)) + except SnipeITApiError as e: + if getattr(e, "status_code", None) not in (404,): + raiseEnsure
SnipeITApiErroris imported at top (it is).tests/integration/resources/test_locations.py (2)
21-25: Avoidgetattrwith a constant; access the attribute directly.Small readability nit; also satisfies Ruff B009.
- else: - assert int(getattr(got_child, "parent_id")) == id_int(root) + else: + assert int(got_child.parent_id) == id_int(root)
36-43: Tighten cleanup exception handling.Ignore only 404; re-raise others.
- try: - c.locations.delete(id_int(child)) - except Exception: - pass + try: + c.locations.delete(id_int(child)) + except SnipeITApiError as e: + if getattr(e, "status_code", None) not in (404,): + raise - try: - c.locations.delete(id_int(root)) - except Exception: - pass + try: + c.locations.delete(id_int(root)) + except SnipeITApiError as e: + if getattr(e, "status_code", None) not in (404,): + raiseAdd import if missing:
from snipeit.exceptions import ( SnipeITNotFoundError, SnipeITApiError, )tests/integration/resources/test_consumables.py (1)
34-37: Narrow cleanup exceptions.Consistent with other tests: ignore 404 only.
- try: - c.consumables.delete(id_int(cons)) - except Exception: - pass + try: + c.consumables.delete(id_int(cons)) + except SnipeITApiError as e: # type: ignore[name-defined] + if getattr(e, "status_code", None) not in (404,): + raiseAdd at top if not already imported:
-from snipeit import SnipeIT +from snipeit import SnipeIT +from snipeit.exceptions import SnipeITApiErrortests/integration/resources/test_status_labels.py (1)
29-32: Narrow cleanup exceptions instead ofexcept Exception.Catching all exceptions can hide real failures during teardown. Limit to expected API errors.
- finally: - try: - c.status_labels.delete(id_int(lab)) - except Exception: - pass + finally: + try: + c.status_labels.delete(id_int(lab)) + except (SnipeITNotFoundError, SnipeITApiError): + passtests/integration/resources/test_accessories.py (1)
45-48: Tighten teardown exception handling.Avoid a blind
except. Restrict to known API exceptions.- finally: - try: - c.accessories.delete(id_int(acc)) - except Exception: - pass + finally: + try: + c.accessories.delete(id_int(acc)) + except (SnipeITNotFoundError, SnipeITApiError): + passtests/integration/resources/test_categories.py (1)
34-37: Avoid catchingExceptionin cleanup.Scope the except to expected API failures.
- finally: - try: - c.categories.delete(id_int(created)) - except Exception: - pass + finally: + try: + c.categories.delete(id_int(created)) + except (SnipeITNotFoundError, SnipeITApiError): + passtests/integration/resources/test_components.py (1)
35-38: Refine teardown exception handling.Don’t swallow unexpected errors.
- finally: - try: - c.components.delete(id_int(comp)) - except Exception: - pass + finally: + try: + c.components.delete(id_int(comp)) + except (SnipeITNotFoundError, SnipeITApiError): + passtests/integration/resources/test_models.py (1)
43-46: Limit broad exception in cleanup.Prefer explicit API exceptions.
- finally: - try: - c.models.delete(id_int(m)) - except Exception: - pass + finally: + try: + c.models.delete(id_int(m)) + except (SnipeITNotFoundError, SnipeITApiError): + passtests/integration/resources/test_departments.py (2)
34-38: Tighten teardown exception handling.Avoid a blanket
except.- finally: - try: - c.departments.delete(id_int(dep)) - except Exception: - pass + finally: + try: + c.departments.delete(id_int(dep)) + except (SnipeITNotFoundError, SnipeITApiError): + pass
39-41: Add post‑delete NotFound assertion for symmetry.Other resource tests assert the created ID is gone after deletion; mirror that here.
- with pytest.raises((SnipeITNotFoundError, SnipeITApiError)): - c.departments.get(99999999) + # Ensure the deleted department is not retrievable + with pytest.raises((SnipeITNotFoundError, SnipeITApiError)): + c.departments.get(id_int(dep)) + + # Non-existent sentinel + with pytest.raises((SnipeITNotFoundError, SnipeITApiError)): + c.departments.get(99999999)tests/integration/resources/test_users.py (2)
19-25: Silence “hardcoded password” lint in tests.These are test creds; annotate to appease Ruff S106 without changing behavior.
- password="Pass1234!", - password_confirmation="Pass1234!", + password="Pass1234!", # noqa: S106 - test credential + password_confirmation="Pass1234!", # noqa: S106 - test credential @@ - password="Pass1234!", - password_confirmation="Pass1234!", + password="Pass1234!", # noqa: S106 - test credential + password_confirmation="Pass1234!", # noqa: S106 - test credentialAlso applies to: 36-43
55-58: Restrict teardown exceptions.Avoid swallowing unexpected issues during cleanup.
- finally: - try: - c.users.delete(id_int(u)) - except Exception: - pass + finally: + try: + c.users.delete(id_int(u)) + except (SnipeITNotFoundError, SnipeITApiError): + passtests/integration/resources/test_assets.py (3)
70-73: Narrow delete cleanup exceptions.Limit to expected API errors.
- finally: - try: - c.assets.delete(id_int(a)) - except Exception: - pass + finally: + try: + c.assets.delete(id_int(a)) + except (SnipeITNotFoundError, SnipeITApiError): + pass
93-107: Narrow exceptions in nested cleanup.Same concern for the nested asset
bdeletion.- finally: - try: - c.assets.delete(id_int(b)) - except Exception: - pass + finally: + try: + c.assets.delete(id_int(b)) + except (SnipeITNotFoundError, SnipeITApiError): + pass
75-87: Optional: exercise restore flow if supported.Given recent additions, consider restoring the asset after delete (soft‑delete path) to validate the new API.
try: after = c.assets.get(id_int(a)) deleted_markers = [ getattr(after, "deleted_at", None), getattr(after, "deleted", None), getattr(after, "archived", None), ] assert any(bool(m) for m in deleted_markers) except (SnipeITNotFoundError, SnipeITApiError): pass + +# If restore is available, try it and assert the asset returns normally again. +if hasattr(c.assets, "restore"): + try: + c.assets.restore(id_int(a)) + restored = c.assets.get(id_int(a)) + assert id_int(restored) == id_int(a) + except (SnipeITApiError, SnipeITNotFoundError): + # Some servers may hard-delete; accept failure. + passtests/unit/test_assets_endpoints.py (4)
2-3: Remove unused imports.
os,io, andSnipeITare unused in this test file.-import os -import io -from snipeit import SnipeITAlso applies to: 6-6
20-20: Drop unnecessary type ignore.
labels()acceptsList[str]; the ignore mask isn’t needed.- out = snipeit_client.assets.labels(str(save_path), ["TAG1"]) # type: ignore[arg-type] + out = snipeit_client.assets.labels(str(save_path), ["TAG1"])
48-50: Assert the refreshed asset to verify workflow.Make the test validate the return from
audit().- asset = snipeit_client.assets._make({"id": 1, "asset_tag": "A1"}) - asset.audit(note="checked") + asset = snipeit_client.assets._make({"id": 1, "asset_tag": "A1"}) + updated = asset.audit(note="checked") + assert updated.id == 1
9-23: Add a unit test for direct PDF responses.You already cover the JSON base64 path; also cover
Content-Type: application/pdf.@pytest.mark.unit def test_labels_writes_pdf_response(snipeit_client, requests_mock, tmp_path): pdf_bytes = b"%PDF-1.4 direct" requests_mock.post( "https://test.snipeitapp.com/api/v1/hardware/labels", content=pdf_bytes, status_code=200, headers={"Content-Type": "application/pdf"}, ) save_path = tmp_path / "labels.pdf" out = snipeit_client.assets.labels(str(save_path), ["TAG2"]) assert out == str(save_path) assert save_path.read_bytes() == pdf_bytestests/integration/conftest.py (2)
8-13: Replace try/except/pass cleanup with targeted suppression.Use
contextlib.suppress(SnipeITException)to avoid catching everything and silence Ruff S110/BLE001.@@ import time import uuid from typing import Dict, Any +import contextlib +from snipeit.exceptions import SnipeITException @@ - try: - c.users.delete(_id_int(user)) - except Exception: - pass - try: - c.models.delete(_id_int(model)) - except Exception: - pass - try: - c.status_labels.delete(_id_int(status_deploy)) - except Exception: - pass - try: - c.status_labels.delete(_id_int(status_undep)) - except Exception: - pass + with contextlib.suppress(SnipeITException): + c.users.delete(_id_int(user)) + with contextlib.suppress(SnipeITException): + c.models.delete(_id_int(model)) + with contextlib.suppress(SnipeITException): + c.status_labels.delete(_id_int(status_deploy)) + with contextlib.suppress(SnipeITException): + c.status_labels.delete(_id_int(status_undep)) @@ - try: - c.locations.delete(_id_int(loc_child)) - except Exception: - pass - try: - c.locations.delete(_id_int(loc_root)) - except Exception: - pass + with contextlib.suppress(SnipeITException): + c.locations.delete(_id_int(loc_child)) + with contextlib.suppress(SnipeITException): + c.locations.delete(_id_int(loc_root)) @@ - for cat in (cat_asset, cat_acc, cat_comp, cat_cons, cat_lic): - try: - c.categories.delete(_id_int(cat)) - except Exception: - pass - try: - c.manufacturers.delete(_id_int(mfg)) - except Exception: - pass + for cat in (cat_asset, cat_acc, cat_comp, cat_cons, cat_lic): + with contextlib.suppress(SnipeITException): + c.categories.delete(_id_int(cat)) + with contextlib.suppress(SnipeITException): + c.manufacturers.delete(_id_int(mfg))Also applies to: 147-181
118-126: Hardcoded password in tests is acceptable; consider marking as test-only.Optional: add an inline comment to quiet linters or add
# noqa: S106on both password lines.snipeit/resources/assets.py (6)
231-241: Preserve response context and exception chaining on upload error.Pass
respintoSnipeITApiErrorand chain JSON decode failures.- if resp.status_code >= 400: + if resp.status_code >= 400: try: - body = resp.json() + body = resp.json() msg = body.get("messages") or body.get("message") or resp.reason except ValueError: msg = resp.text or resp.reason - raise SnipeITApiError(str(msg)) + raise SnipeITApiError(str(msg), resp) try: - return resp.json() - except ValueError: - raise SnipeITApiError("Expected JSON response from file upload") + return resp.json() + except ValueError as e: + raise SnipeITApiError("Expected JSON response from file upload") from e
253-260: Include response on download errors.Propagate
respto keepstatus_code/body for callers.- if resp.status_code != 200: + if resp.status_code != 200: try: body = resp.json() msg = body.get("messages") or body.get("message") or resp.reason except ValueError: msg = resp.text or resp.reason - raise SnipeITApiError(str(msg)) + raise SnipeITApiError(str(msg), resp)
251-265: Stream downloads to avoid large-memory reads.Use
stream=Trueand chunked writes.- resp = self.api.session.get(url, timeout=self.api.timeout) + resp = self.api.session.get(url, timeout=self.api.timeout, stream=True) @@ - with open(save_path, "wb") as f: - f.write(resp.content) + with open(save_path, "wb") as f: + for chunk in resp.iter_content(chunk_size=8192): + if chunk: + f.write(chunk)
308-315: Preserve response on labels errors.Pass
respinto the error for better diagnostics.- if resp.status_code >= 400: + if resp.status_code >= 400: try: body = resp.json() msg = body.get("messages") or body.get("message") or resp.reason except ValueError: msg = resp.text or resp.reason - raise SnipeITApiError(str(msg)) + raise SnipeITApiError(str(msg), resp)
343-346: Validate base64 input and chain the original error.Use
validate=Trueandfrom e.- try: - pdf_bytes = base64.b64decode(b64) - except Exception as e: - raise SnipeITApiError(f"Failed to decode label file: {e}") + try: + pdf_bytes = base64.b64decode(b64, validate=True) + except Exception as e: + raise SnipeITApiError(f"Failed to decode label file: {e}") from e
188-189: Clarify unexpected byserial response.Include type info to aid debugging.
- raise SnipeITApiError("Unexpected response for byserial") + raise SnipeITApiError(f"Unexpected response for byserial: {type(response).__name__}")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
.gitignore(1 hunks)pyproject.toml(1 hunks)snipeit/resources/assets.py(7 hunks)tests/integration/conftest.py(2 hunks)tests/integration/resources/test_accessories.py(1 hunks)tests/integration/resources/test_assets.py(1 hunks)tests/integration/resources/test_assets_checkout_integration.py(0 hunks)tests/integration/resources/test_assets_integration.py(0 hunks)tests/integration/resources/test_categories.py(1 hunks)tests/integration/resources/test_components.py(1 hunks)tests/integration/resources/test_consumables.py(1 hunks)tests/integration/resources/test_departments.py(1 hunks)tests/integration/resources/test_fields.py(1 hunks)tests/integration/resources/test_fieldsets.py(1 hunks)tests/integration/resources/test_licenses.py(1 hunks)tests/integration/resources/test_locations.py(1 hunks)tests/integration/resources/test_manufacturers.py(1 hunks)tests/integration/resources/test_models.py(1 hunks)tests/integration/resources/test_status_labels.py(1 hunks)tests/integration/resources/test_users.py(1 hunks)tests/unit/test_assets_endpoints.py(1 hunks)
💤 Files with no reviewable changes (2)
- tests/integration/resources/test_assets_checkout_integration.py
- tests/integration/resources/test_assets_integration.py
🧰 Additional context used
🧬 Code graph analysis (17)
tests/integration/resources/test_fieldsets.py (5)
snipeit/client.py (1)
SnipeIT(18-200)snipeit/exceptions.py (1)
SnipeITApiError(19-30)tests/conftest.py (1)
real_snipeit_client(13-26)tests/integration/conftest.py (3)
run_id(61-63)_n(67-72)id_int(76-78)snipeit/resources/base.py (1)
save(55-80)
tests/integration/resources/test_departments.py (5)
snipeit/client.py (1)
SnipeIT(18-200)snipeit/exceptions.py (2)
SnipeITNotFoundError(38-40)SnipeITApiError(19-30)tests/conftest.py (1)
real_snipeit_client(13-26)tests/integration/conftest.py (3)
run_id(61-63)_n(67-72)id_int(76-78)snipeit/resources/base.py (1)
save(55-80)
tests/integration/resources/test_licenses.py (4)
snipeit/client.py (1)
SnipeIT(18-200)tests/conftest.py (1)
real_snipeit_client(13-26)tests/integration/conftest.py (4)
base(82-181)run_id(61-63)_n(67-72)id_int(76-78)snipeit/resources/base.py (1)
save(55-80)
tests/integration/resources/test_locations.py (4)
snipeit/client.py (1)
SnipeIT(18-200)snipeit/exceptions.py (2)
SnipeITNotFoundError(38-40)SnipeITApiError(19-30)tests/conftest.py (1)
real_snipeit_client(13-26)tests/integration/conftest.py (3)
run_id(61-63)_n(67-72)id_int(76-78)
tests/integration/resources/test_categories.py (4)
snipeit/client.py (1)
SnipeIT(18-200)snipeit/exceptions.py (2)
SnipeITNotFoundError(38-40)SnipeITApiError(19-30)tests/conftest.py (1)
real_snipeit_client(13-26)tests/integration/conftest.py (3)
run_id(61-63)_n(67-72)id_int(76-78)
tests/integration/resources/test_accessories.py (6)
snipeit/client.py (1)
SnipeIT(18-200)snipeit/exceptions.py (4)
SnipeITNotFoundError(38-40)SnipeITValidationError(43-58)SnipeITClientError(61-63)SnipeITApiError(19-30)tests/conftest.py (1)
real_snipeit_client(13-26)tests/integration/conftest.py (4)
base(82-181)run_id(61-63)_n(67-72)id_int(76-78)snipeit/resources/base.py (1)
save(55-80)snipeit/resources/accessories.py (1)
checkin_from_user(40-55)
tests/integration/resources/test_fields.py (5)
snipeit/client.py (1)
SnipeIT(18-200)snipeit/exceptions.py (2)
SnipeITNotFoundError(38-40)SnipeITApiError(19-30)tests/conftest.py (1)
real_snipeit_client(13-26)tests/integration/conftest.py (3)
run_id(61-63)_n(67-72)id_int(76-78)snipeit/resources/base.py (1)
save(55-80)
tests/integration/resources/test_models.py (5)
snipeit/client.py (1)
SnipeIT(18-200)snipeit/exceptions.py (2)
SnipeITNotFoundError(38-40)SnipeITApiError(19-30)tests/conftest.py (1)
real_snipeit_client(13-26)tests/integration/conftest.py (4)
base(82-181)run_id(61-63)_n(67-72)id_int(76-78)snipeit/resources/base.py (1)
save(55-80)
tests/integration/resources/test_consumables.py (3)
snipeit/client.py (1)
SnipeIT(18-200)tests/conftest.py (1)
real_snipeit_client(13-26)tests/integration/conftest.py (4)
base(82-181)run_id(61-63)_n(67-72)id_int(76-78)
tests/unit/test_assets_endpoints.py (3)
tests/conftest.py (1)
snipeit_client(7-9)snipeit/resources/assets.py (12)
labels(272-349)audit_by_id(122-124)audit(69-84)list_audit_overdue(126-128)list_audit_due(130-132)restore(86-90)get_licenses(206-208)list_files(211-213)upload_files(215-247)download_file(249-265)delete_file(267-269)get_by_serial(153-188)snipeit/resources/base.py (1)
_make(147-148)
tests/integration/resources/test_users.py (5)
snipeit/client.py (1)
SnipeIT(18-200)snipeit/exceptions.py (3)
SnipeITValidationError(43-58)SnipeITNotFoundError(38-40)SnipeITApiError(19-30)tests/conftest.py (1)
real_snipeit_client(13-26)tests/integration/conftest.py (3)
run_id(61-63)_n(67-72)id_int(76-78)snipeit/resources/users.py (1)
me(34-41)
tests/integration/resources/test_manufacturers.py (4)
snipeit/exceptions.py (2)
SnipeITNotFoundError(38-40)SnipeITApiError(19-30)tests/conftest.py (1)
real_snipeit_client(13-26)tests/integration/conftest.py (3)
run_id(61-63)_n(67-72)id_int(76-78)snipeit/resources/base.py (2)
list_all(162-183)save(55-80)
tests/integration/resources/test_assets.py (6)
snipeit/client.py (1)
SnipeIT(18-200)snipeit/exceptions.py (4)
SnipeITNotFoundError(38-40)SnipeITValidationError(43-58)SnipeITClientError(61-63)SnipeITApiError(19-30)tests/conftest.py (1)
real_snipeit_client(13-26)tests/integration/conftest.py (4)
base(82-181)run_id(61-63)_n(67-72)id_int(76-78)snipeit/resources/base.py (1)
save(55-80)snipeit/resources/assets.py (6)
get_by_tag(134-151)get_by_serial(153-188)checkout(26-53)checkin(55-67)audit(69-84)labels(272-349)
tests/integration/resources/test_components.py (4)
snipeit/client.py (1)
SnipeIT(18-200)tests/conftest.py (1)
real_snipeit_client(13-26)tests/integration/conftest.py (4)
base(82-181)run_id(61-63)_n(67-72)id_int(76-78)snipeit/resources/base.py (1)
save(55-80)
tests/integration/resources/test_status_labels.py (4)
snipeit/client.py (1)
SnipeIT(18-200)snipeit/exceptions.py (2)
SnipeITNotFoundError(38-40)SnipeITApiError(19-30)tests/conftest.py (1)
real_snipeit_client(13-26)tests/integration/conftest.py (3)
run_id(61-63)_n(67-72)id_int(76-78)
tests/integration/conftest.py (2)
snipeit/client.py (1)
SnipeIT(18-200)tests/conftest.py (1)
real_snipeit_client(13-26)
snipeit/resources/assets.py (3)
snipeit/resources/base.py (6)
_create(116-118)refresh(82-90)_get(112-114)get(185-189)_make(147-148)_delete(124-127)snipeit/exceptions.py (2)
SnipeITNotFoundError(38-40)SnipeITApiError(19-30)snipeit/client.py (2)
get(179-181)post(183-185)
🪛 Ruff (0.12.2)
tests/integration/resources/test_fieldsets.py
37-38: try-except-pass detected, consider logging the exception
(S110)
37-37: Do not catch blind exception: Exception
(BLE001)
tests/integration/resources/test_departments.py
37-38: try-except-pass detected, consider logging the exception
(S110)
37-37: Do not catch blind exception: Exception
(BLE001)
tests/integration/resources/test_licenses.py
36-37: try-except-pass detected, consider logging the exception
(S110)
36-36: Do not catch blind exception: Exception
(BLE001)
tests/integration/resources/test_locations.py
24-24: Do not call getattr with a constant attribute value. It is not any safer than normal property access.
Replace getattr with attribute access
(B009)
38-39: try-except-pass detected, consider logging the exception
(S110)
38-38: Do not catch blind exception: Exception
(BLE001)
42-43: try-except-pass detected, consider logging the exception
(S110)
42-42: Do not catch blind exception: Exception
(BLE001)
tests/integration/resources/test_categories.py
36-37: try-except-pass detected, consider logging the exception
(S110)
36-36: Do not catch blind exception: Exception
(BLE001)
tests/integration/resources/test_accessories.py
47-48: try-except-pass detected, consider logging the exception
(S110)
47-47: Do not catch blind exception: Exception
(BLE001)
tests/integration/resources/test_fields.py
37-38: try-except-pass detected, consider logging the exception
(S110)
37-37: Do not catch blind exception: Exception
(BLE001)
tests/integration/resources/test_models.py
45-46: try-except-pass detected, consider logging the exception
(S110)
45-45: Do not catch blind exception: Exception
(BLE001)
tests/integration/resources/test_consumables.py
36-37: try-except-pass detected, consider logging the exception
(S110)
36-36: Do not catch blind exception: Exception
(BLE001)
tests/integration/resources/test_users.py
23-23: Possible hardcoded password assigned to argument: "password"
(S106)
24-24: Possible hardcoded password assigned to argument: "password_confirmation"
(S106)
41-41: Possible hardcoded password assigned to argument: "password"
(S106)
42-42: Possible hardcoded password assigned to argument: "password_confirmation"
(S106)
57-58: try-except-pass detected, consider logging the exception
(S110)
57-57: Do not catch blind exception: Exception
(BLE001)
tests/integration/resources/test_manufacturers.py
39-40: try-except-pass detected, consider logging the exception
(S110)
39-39: Do not catch blind exception: Exception
(BLE001)
tests/integration/resources/test_assets.py
72-73: try-except-pass detected, consider logging the exception
(S110)
72-72: Do not catch blind exception: Exception
(BLE001)
105-106: try-except-pass detected, consider logging the exception
(S110)
105-105: Do not catch blind exception: Exception
(BLE001)
tests/integration/resources/test_components.py
37-38: try-except-pass detected, consider logging the exception
(S110)
37-37: Do not catch blind exception: Exception
(BLE001)
tests/integration/resources/test_status_labels.py
31-32: try-except-pass detected, consider logging the exception
(S110)
31-31: Do not catch blind exception: Exception
(BLE001)
tests/integration/conftest.py
124-124: Possible hardcoded password assigned to argument: "password"
(S106)
125-125: Possible hardcoded password assigned to argument: "password_confirmation"
(S106)
149-150: try-except-pass detected, consider logging the exception
(S110)
149-149: Do not catch blind exception: Exception
(BLE001)
153-154: try-except-pass detected, consider logging the exception
(S110)
153-153: Do not catch blind exception: Exception
(BLE001)
157-158: try-except-pass detected, consider logging the exception
(S110)
157-157: Do not catch blind exception: Exception
(BLE001)
161-162: try-except-pass detected, consider logging the exception
(S110)
161-161: Do not catch blind exception: Exception
(BLE001)
166-167: try-except-pass detected, consider logging the exception
(S110)
166-166: Do not catch blind exception: Exception
(BLE001)
170-171: try-except-pass detected, consider logging the exception
(S110)
170-170: Do not catch blind exception: Exception
(BLE001)
176-177: try-except-pass detected, consider logging the exception
(S110)
176-176: Do not catch blind exception: Exception
(BLE001)
180-181: try-except-pass detected, consider logging the exception
(S110)
180-180: Do not catch blind exception: Exception
(BLE001)
snipeit/resources/assets.py
176-176: Avoid specifying long messages outside the exception class
(TRY003)
181-181: Avoid specifying long messages outside the exception class
(TRY003)
182-182: Avoid specifying long messages outside the exception class
(TRY003)
188-188: Avoid specifying long messages outside the exception class
(TRY003)
218-218: Avoid specifying long messages outside the exception class
(TRY003)
241-241: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
241-241: Avoid specifying long messages outside the exception class
(TRY003)
246-247: try-except-pass detected, consider logging the exception
(S110)
246-246: Do not catch blind exception: Exception
(BLE001)
330-330: Avoid specifying long messages outside the exception class
(TRY003)
341-341: Avoid specifying long messages outside the exception class
(TRY003)
345-345: Do not catch blind exception: Exception
(BLE001)
346-346: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
346-346: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (11)
tests/integration/resources/test_licenses.py (1)
24-26: Confirmed — ApiObject tracks dirty fields and save() PATCHes only changed fields.ApiObject.setattr adds modified public attributes to _dirty_fields and save() builds/sends a PATCH from those fields and clears the set on success (snipeit/resources/base.py).
tests/integration/resources/test_status_labels.py (1)
14-28: CRUD flow looks solid.Happy‑path coverage, patch/get verification, and list smoke check are all good.
tests/integration/resources/test_accessories.py (1)
16-44: Good end‑to‑end coverage, including negative path.Create/patch/save/list flows and the invalid checkin path are exercised well.
tests/integration/resources/test_categories.py (1)
14-33: LGTM on CRUD assertions.Covers create/get/list/update and post‑delete NotFound checks cleanly.
tests/integration/resources/test_components.py (1)
10-33: Component CRUD path looks good.Assertions around qty via patch/save and list smoke check are appropriate.
tests/integration/resources/test_models.py (1)
14-41: Solid coverage of model_number updates via patch and save.End‑to‑end behavior is exercised well.
tests/integration/resources/test_departments.py (1)
14-33: CRUD path reads cleanly.Patch, save, and list checks are fine.
tests/integration/resources/test_users.py (1)
15-33: Users CRUD + me() coverage is on point.Good validation of duplicate username behavior and token owner via me().
tests/integration/resources/test_assets.py (2)
19-49: Great full‑flow asset coverage.Covers tag/serial lookups, checkout/checkin, audit, label generation, and list presence. Nice handling for differing server behavior on labels.
66-69: Potential flake: paginated list().If
list()is paginated, the new asset may not appear on the first page. Consider passing a search filter (e.g.,search=a.asset_tag) or a higherlimitif supported by the manager to reduce flakiness.Would you confirm the
AssetsManager.listsignature supportssearch/limitand apply the same pattern across resource list “smoke” checks if available?snipeit/resources/assets.py (1)
86-91: Overall: Assets API expansions look solid.Good coverage of audits, restore, by-serial envelope handling, labels, and file ops; API paths align with tests; refresh usage is consistent.
If you want, I can add negative-path unit tests (4xx/5xx for uploads/downloads/labels) to ensure error propagation behaves as intended after the above tweaks.
Also applies to: 121-133, 205-270, 271-349
Docstrings generation was requested by @wilcollier. * #8 (comment) The following files were modified: * `snipeit/resources/assets.py` * `tests/integration/conftest.py` * `tests/integration/resources/test_components.py` * `tests/integration/resources/test_consumables.py` * `tests/integration/resources/test_fieldsets.py` * `tests/integration/resources/test_locations.py` * `tests/integration/resources/test_models.py`
|
Note Generated docstrings for this pull request at #9 |
wilcollier
changed the title
1 participant
Summary by CodeRabbit
New Features
Tests
Chores