Skip to content

Conversation

@jorwoods
Copy link
Contributor

  • feat: List server extension settings
  • feat: support updating server extension settings
  • feat: enable retrieving site extension settings
  • feat: support updating site settings

Closes tableau#1629

New attributes have been added to extension site settings for
upcoming Tableau Cloud releases.
@jorwoods jorwoods force-pushed the jorwoods/extensions branch from ea7ba86 to bcea6f4 Compare October 21, 2025 03:42
@jorwoods
Copy link
Contributor Author

@bcantoni @jacksonlauren

I also added the new attributes as referenced in #1629, but do not have the .xsd file for the future release that will reflect those attributes so cannot verify the structure of what I have added. Please review the structure I had assumed based on that issue.

@bcantoni
Copy link
Contributor

thanks - I'll get one of the right engineers to review!

@jacalata jacalata requested a review from Copilot October 29, 2025 04:42
Copy link

Copilot AI left a 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 adds support for managing Tableau extensions settings at both server and site levels. The implementation includes new model classes for extensions configuration, API endpoints for retrieving and updating settings, and comprehensive test coverage.

Key changes:

  • Added new model classes (ExtensionsServer, ExtensionsSiteSettings, SafeExtension) to represent extension settings
  • Implemented Extensions endpoint with methods for getting and updating server and site-level extension settings
  • Added test coverage for all extension-related operations

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
tableauserverclient/models/extensions_item.py Defines data models for extensions server settings, site settings, and safe extensions
tableauserverclient/server/endpoint/extensions_endpoint.py Implements API endpoint for managing extensions settings
tableauserverclient/server/request_factory.py Adds request builders for extensions update operations
tableauserverclient/server/server.py Integrates Extensions endpoint into Server class
tableauserverclient/server/endpoint/init.py Exports Extensions endpoint
tableauserverclient/models/init.py Exports extensions model classes
tableauserverclient/init.py Adds extensions classes to public API
tableauserverclient/models/property_decorators.py Adds Tuple import for type hints
test/test_extensions.py Comprehensive test suite for extensions functionality
test/assets/extensions_site_settings.xml Test fixture for site-level extension settings
test/assets/extensions_server_settings_true.xml Test fixture for enabled server extensions
test/assets/extensions_server_settings_false.xml Test fixture for disabled server extensions

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

Copy link

@jacksonlauren jacksonlauren left a comment

Choose a reason for hiding this comment

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

Thank you for this!! Looking great, just a few questions (mostly for Brian and team)

Comment on lines +94 to +101
@property
def use_default_setting(self) -> bool | None:
return self._use_default_setting

@use_default_setting.setter
@property_is_boolean
def use_default_setting(self, value: bool | None) -> None:
self._use_default_setting = value

Choose a reason for hiding this comment

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

I believe the useDefaultSetting setting is deprecated by the time new new trusted extensions settings come around - we can skip adding it I think! @bcantoni to confirm

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jacksonlauren Sounds like we might need to keep this for as long as anything <3.27 is supported though.

Choose a reason for hiding this comment

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

If I'm remembering correctly, it is fully unhooked from any client code - essentially a noop - and has been for some time now. Even though <3.27 has it in the API, there's not a need for anyone to use it.

I'll check with Brian though, I'm not sure when exactly that happened and if we still support versions of Tableau that would utilize useDefaultSetting in any way

Comment on lines +63 to +65
def update(self, extensions_site_settings: ExtensionsSiteSettings) -> ExtensionsSiteSettings:
"""Updates the extensions settings for the site. Overwrites all existing settings.
Any extensions omitted from the safe extensions list will be removed.

Choose a reason for hiding this comment

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

With the API directly, all of the settings are optional, and only overwrite if included (with the safelist being a full overwrite iff included), but it seems here that that is not what this comment is.

Is that an intentional change for the python wrapping? Or am I misreading the comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I'll update the docstring to reflect that. I over-generalized the behavior from the safelist

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jacksonlauren How would the user purge the safe list? The behavior I wrote would not add any safeList sub elements if the user sets it as an empty list.

https://github.com/tableau/server-client-python/pull/1672/files#diff-d4293e36603783537f9002365e19cc03c973fe1e68b4192d4466bbf0bc555c95R1666-R1667

Copy link

@jacksonlauren jacksonlauren Oct 30, 2025

Choose a reason for hiding this comment

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

With the API directly, you'd pass in an empty array to designate purging the list. So for this instance I think we'd want something like this, if the aim is to match the REST API:

if extensions_site_settings.safe_list is not None: 
    /* purge the list here */

    for safe in extensions_site_settings.safe_list:
        /* add in the new elements */

/* implied else => do nothing to the safe list */

response = self.put_request(self._server_baseurl, req)
return ExtensionsServer.from_response(response.content, self.parent_srv.namespace)

@api(version="3.21")

Choose a reason for hiding this comment

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

For the site settings, I wonder if we should just use 3.27 here? The methods were added in 3.21, but the new trusted items and such weren't added until 3.27. Thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants