Fix publish failures when using embedded db credentials - #458
Conversation
There was a problem hiding this comment.
Pull request overview
Attempts to fix credentialed workbook publishing for TSC 0.41.
Changes:
- Adds
--db-serverand assigns it to workbook connections. - Passes workbook connections as a list and adds tests.
- Adds localized help and recompiles catalogs.
Reviewed changes
Copilot reviewed 14 out of 25 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tabcmd/commands/datasources_and_workbooks/publish_command.py |
Builds and passes workbook connections. |
tabcmd/execution/global_options.py |
Adds --db-server. |
tests/commands/test_publish_command.py |
Tests connection payloads. |
tabcmd/locales/{de,en,es,fr,ga,it,ja,ko,pt,sv,zh}/tabcmd_messages_*.properties |
Adds localized option help. |
tabcmd/locales/{de,en,es,fr,ga,it,ja,ko,pt,sv,zh}/LC_MESSAGES/tabcmd.mo |
Updates compiled catalogs. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if args.db_username: | ||
| creds = TSC.models.ConnectionCredentials(args.db_username, args.db_password, embed=args.save_db_password) | ||
| workbook_connections = TSC.ConnectionItem() | ||
| workbook_connections.server_address = args.db_server |
There was a problem hiding this comment.
This comment is correct. You need to update your test to fail when no server is given - if you also want to add the check in run_command that'd be great, but it's not necessary before this gets checked in.
| publish.options.append=Anexe o arquivo de extração à fonte de dados existente | ||
| publish.options.async=Publicar de forma assíncrona | ||
| publish.options.db-password=Senha do banco de dados para todas as fontes de dados | ||
| publish.options.db-server=Endereço do servidor do banco de dado associado as credenciais. Obrigatório ao usar --db-username ou --oauth-username. |
There was a problem hiding this comment.
This is fine unless you want to make the edits. I will do the package regeneration with new messages.
|
Sorry for the slow response - one change needed to the test mentioned, otherwise it looks great. |
Summary
--db-username/--db-password/--save-db-passwordor--oauth-usernamealways failed withValueError: Connection must have a server address, because theConnectionItembuilt for the workbook'sconnections=payload never setserver_address.tableauserverclient0.41 now hard-requires it.publish_workbook_file()passed the bareConnectionIteminstead of wrapping it in a list, causingTypeError: object of type 'ConnectionItem' has no len().--db-serverCLI flag so users can supply the database server address to associate with the embedded connection credentials. When omitted,server_addressis left unset (None), matching the flag's optional nature.publish_workbook_file()to passconnections=[credentials](a list) instead of the bareConnectionItem, matching whattableauserverclientexpects.publish.options.db-server) across all supported locales (de, en, es, fr, ga, it, ja, ko, pt, sv, zh), including recompiled.mocatalogs.Test plan
python -m pytest tests/ -v— 318 passed, 2 skipped (pre-existing, unrelated), 0 failedpython -m pytest tests/commands/test_publish_command.py -v— 10/10 passing, including coverage forserver_addressbeing set from--db-serverand for theNonecase, and for the wrapped-listconnectionspayloadtabcmd publish --helprenders the new--db-serverflag and localized help text (spot-checkedenandde/ptviaLANGoverride)server.workbooks.publishcall)