Skip to content

Conversation

@psyclaudeZ
Copy link
Contributor

@psyclaudeZ psyclaudeZ commented Oct 13, 2025

Before this PR

Addressing issue #19, implicitly addressing issue #55.

Positional args are not ideal because, combining with the fact that conjure-python alphabetically sorts the arg list, the approach is just not set up for backward compatibility. Whenever a new optional (otherwise it's deemed a breaking change) item is added to the Conjure definiton, the dev better hopes the generated API isn't invoked positionally otherwise the consuming callsites will likely break. For the back compat, they need to make sure the generated argument ends up at the very end of the arg list (sometimes involving wittedly naming the variable which is sad and I have personally had the convo internally) so that they can get assigned a default None properly. Additionally, when the list is too long, the invoking code could be barely readable in positional style (without comments). And if there are comments, it's effectively keyword style invocation.

Why can't we just not sort the arg list and keep the rest status quo? Because even though it does address most of the back compat issues, it can still occur if the item is somehow added to the middle of the Conjure definitions and the callsites are positionally invoked. Enforcing kwargs will get rid of this headache once and for all. And practically, I don't think this will involve a lot of changes on the consumers' end (see Possible downsides below), making it a pretty good ROI.

After this PR

Introducing a CLI flag that determines whether to force kwargs or not. The generated code of targets are bean type constructor args, union type constructor args, and API endpoints.

The particular changes are mostly threading through the callstacks, logic for generating '*', and unit test.

Disclaimer: I used Claude Code to create just the boilerplate of the unit test, cause I don't like plumbing work ;)

==COMMIT_MSG==
Add an option to force keyword arguments
==COMMIT_MSG==

Possible downsides?

For now, the flag defaults to false so it's fully backward compatible.

However, going forward we do plan to enforce it in the next major release. This means all the consumers for the affected generated code will need to be invoked keyword-only. However, 1) Anecdotally, my observation internally suggests that majority (if not all) of the consumers of the generated code are already using kwargs invocation 2) Given the back compat trouble mentioned earlier, it's just not safe to use positional style unless it's a oneoff, so some people might already realize this and are doing kwargs invocations. TLDR it's either you're effectively already doing it or what you're doing is unsafe and you better move off of it situation. And we should enforce the better practice, but definitely with considerate rollout.

@changelog-app
Copy link

changelog-app bot commented Oct 13, 2025

Successfully generated changelog entry!

What happened?

Your changelog entries have been stored in the database as part of our migration to ChangelogV3.

Entry generated via PR title

To modify this entry, edit PR title using proper format.

@psyclaudeZ psyclaudeZ marked this pull request as ready for review October 13, 2025 22:28
@psyclaudeZ psyclaudeZ requested a review from johnhany97 October 13, 2025 22:29
@psyclaudeZ psyclaudeZ requested a review from dtobin October 23, 2025 21:11
@saakshaat-singh saakshaat-singh self-requested a review October 24, 2025 00:53
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