-
Notifications
You must be signed in to change notification settings - Fork 11
Use case-insensitive prefixed logicalName in all operations #39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
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 refactors the Dataverse SDK to enforce consistent naming conventions by normalizing logical names to lowercase and implementing case-insensitive lookups throughout the codebase. The changes introduce a unified entity metadata cache, consolidate helper functions into a shared test utilities module, and update all documentation and examples to reflect the new logical-name-first approach.
- Introduced
_normalize_logical_name()for case-insensitive logical name handling - Consolidated entity metadata caching into a unified
TableMetadataTypedDict structure - Updated all table/column operations to accept logical names with publisher prefixes
- Refactored test helpers into shared
tests/unit/test_helpers.pymodule
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_helpers.py | New shared test utilities module providing mock authentication, HTTP clients, and reusable test data |
| tests/unit/data/test_naming_normalization.py | Comprehensive test suite for logical name normalization and case-insensitive caching |
| tests/unit/data/test_logical_crud.py | Refactored to use shared test helpers instead of duplicated mock classes |
| tests/unit/data/test_enum_optionset_payload.py | Updated to import DummyAuth and DummyConfig from shared test_helpers |
| tests/unit/core/test_http_errors.py | Updated to import DummyAuth and DummyHTTPClient from shared test_helpers |
| src/dataverse_sdk/data/odata.py | Core implementation of logical name normalization, unified metadata cache, and case-insensitive operations |
| src/dataverse_sdk/client.py | Updated public API documentation to reflect logical-name-first naming convention |
| examples/basic/quickstart.py | Updated to use lowercase logical names with publisher prefix throughout |
| examples/advanced/file_upload.py | Updated table creation to match new naming conventions |
| README.md | Updated API reference table to document logical-name-first approach |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
….com/microsoft/PowerPlatform-DataverseClient-Python into user/tpellissier/logical_name_prefix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
….com/microsoft/PowerPlatform-DataverseClient-Python into user/tpellissier/logical_name_prefix
| # Escape single quotes in logical name | ||
| logical_escaped = self._escape_odata_quotes(logical_name) | ||
| params = { | ||
| "$select": "MetadataId,LogicalName,SchemaName,EntitySetName", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we also store metadataid in cache? in general just anything that seems commonly used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and if we do that we can add a cache check inside this function so we dont need to make a call to get these values everytime
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a follow up item here would be good to get our caching stuff better-organized. Some quick thoughts:
- I think caching metadataId is a good idea
- In some cases, getting metadata from the cache makes a lot of sense. But there are some cases where we call this method with consistency-strong because we want to fetch newly created columns or other recently-updated metadata. In those cases we don't want to fetch from cache (not our own SDK cache, and not even the server side cache). So it might be best to have 2 helpers, one for getting metadata info that we don't expect to change (like metadataid, schemaname, etc) and another for force-checking the server metadata and fetching column information etc.
- If we're making those changes, that's a good opportunity to expose a user-facing method to flush our internal metadata caches (just re-init the dictionaries, probably) in case any of the 'static' metadata like schemaname etc. actually does change.
Probably this is best handled in a separate item.
|
This is a reminder to update ReadMe or other doc files to account for changes in this PR to various functions. |
This pull request refactors the Dataverse SDK's table and column management to consistently use logical names with publisher prefixes (e.g.,
new_sampleitem,new_code) instead of friendly or schema names. The changes affect both documentation and code, enforcing stricter naming conventions and improving case-insensitive lookups. The update also simplifies metadata caching and usage throughout the SDK.API and Documentation Updates
create_table,create_columns,get_table_info,list_tables,delete_table,delete_columns) now require logical names with publisher prefixes, and documentation/examples have been updated to reflect this. Friendly names and schema names are no longer accepted. [1] [2] [3] [4]Codebase Refactoring
TableMetadataTypedDictfor efficient lookups. [1] [2]_normalize_logical_nameto ensure all logical names are treated case-insensitively, preventing errors from inconsistent casing.Examples and Usage
README.md,quickstart.py,file_upload.py) now use logical names with publisher prefixes for table and column operations, demonstrating the new conventions. [1] [2] [3] [4]Behavioral Changes
list_tables()now returns a list of logical names instead of dictionaries with schema/friendly names.Error Handling