Skip to content

Make ServerTraceServices.start() plugins argument optional (regression from #68)#73

Merged
pcisar merged 1 commit into
FirebirdSQL:masterfrom
fdcastel:fix-trace-start-plugins-default
May 22, 2026
Merged

Make ServerTraceServices.start() plugins argument optional (regression from #68)#73
pcisar merged 1 commit into
FirebirdSQL:masterfrom
fdcastel:fix-trace-start-plugins-default

Conversation

@fdcastel
Copy link
Copy Markdown
Member

Problem

ServerTraceServices.start() cannot be called the way it always has been:

server.trace.start(config=trace_config, name='my_trace')

raises:

TypeError: ServerTraceServices.start() missing 1 required keyword-only argument: 'plugins'

This breaks every existing caller, including this repository's own
tests/test_server.py::test_trace.

Root cause

#68 added a plugins keyword-only parameter to start(), but without a
default value, so it became required:

def start(self, *, config: str, name: str | None=None, plugins: str | list[str] | None) -> int:

The method body already treats it as optional (if plugins is not None:) and
the docstring documents it as "only for FIREBIRD 6+", so it was clearly
intended to be optional.

Fix

Restore the = None default:

def start(self, *, config: str, name: str | None=None, plugins: str | list[str] | None=None) -> int:

Testing

Adds test_trace_start_plugins_is_optional, a server-free regression test
asserting plugins keeps a default of None. It fails on master and passes
with this change; the existing test_trace integration test also passes again.

FirebirdSQL#68 added a `plugins` keyword-only parameter to
ServerTraceServices.start() but without a default value, making it
required. This breaks every existing caller -- including this project's
own tests/test_server.py::test_trace -- with:

    TypeError: ServerTraceServices.start() missing 1 required
    keyword-only argument: 'plugins'

The method body already treats the argument as optional
(`if plugins is not None:`) and its docstring documents it as FB6+ only,
so restore the `= None` default. Adds a regression test asserting the
parameter keeps a default of None.
@pcisar pcisar merged commit e780ca9 into FirebirdSQL:master May 22, 2026
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