Skip to content

feat(assets): add asset history API methods#79

Open
briansumma wants to merge 2 commits intoNorthShoreAutomation:mainfrom
briansumma:feat/asset-history-api
Open

feat(assets): add asset history API methods#79
briansumma wants to merge 2 commits intoNorthShoreAutomation:mainfrom
briansumma:feat/asset-history-api

Conversation

@briansumma
Copy link
Copy Markdown
Contributor

Asset History API Implementation

Overview

This PR adds support for the asset history API endpoints to the Pythonik SDK. It includes methods to fetch assets, retrieve history entities for a specific asset, and create new history entries.

Changes

  • Added fetch() method to retrieve paginated list of assets
  • Added fetch_asset_history_entities() to get history for an asset
  • Added create_history_entity() to record new history events
  • Implemented comprehensive test suite with proper mocking

Testing

All new functionality is covered by tests in pythonik/tests/test_assets_history.py:

  • Tests for asset fetching
  • Tests for history entity retrieval
  • Tests for creating history entries
  • Error handling for invalid operation types

Note to Maintainer

These changes extend the SDK's capabilities to work with the asset history API without breaking existing functionality. Let me know if any adjustments are needed before merging.

- Add fetch method to retrieve paginated asset list
- Add fetch_asset_history_entities to get history for an asset
- Add create_history_entity to record new history events
- Implement comprehensive test suite for all new methods
)
return self.parse_response(response, None)

def fetch(self, **kwargs) -> Response:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can rename this to list, so its more descriptive of what the intention is? Thanks!

resp = self._get(self.gen_url("assets/"), **kwargs)
return self.parse_response(resp, PaginatedResponse)

def fetch_asset_history_entities(self, asset_id: str, **kwargs) -> Response:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

list_asset_history_entities might make sense here too :)

Raises:
ValueError: If operation_type is not a valid operation type
"""
operation_types = [
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This might be better as an Enum and we can set the operation_type as the Enum, so then the developer/user can use the Enum when sending, also we may not want to restrict them too much here, but just recommend passing the enum or a string, allowing any value but the Enum will help them to see what is expected (this way if the api changes they can still use it)

…t of objects. Refactor `fetch()` be `list_all()` to avoid conflict with `list` built-in.
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