Skip to content
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

[python] always respect typespec namesapce even if package-name is set #5649

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

msyyc
Copy link
Contributor

@msyyc msyyc commented Jan 17, 2025

We have discussed that we shall always respect typespace namespace even if the package-name is different with it.

@microsoft-github-policy-service microsoft-github-policy-service bot added the emitter:client:python Issue for the Python client emitter: @typespec/http-client-python label Jan 17, 2025
@azure-sdk
Copy link
Collaborator

No changes needing a change description found.

@azure-sdk
Copy link
Collaborator

azure-sdk commented Jan 17, 2025

You can try these changes here

🛝 Playground 🌐 Website 📚 Next docs 🛝 VSCode Extension

@msyyc msyyc changed the title [python] always respect typespec namesapce even if package-name is set [python] always respect typespec namesapce even if package-name is set (not ready to review) Jan 17, 2025
@msyyc msyyc changed the title [python] always respect typespec namesapce even if package-name is set (not ready to review) [python] always respect typespec namesapce even if package-name is set Jan 17, 2025
@msyyc msyyc force-pushed the update-root-namespace branch from de51480 to 82afed4 Compare January 20, 2025 07:09
{
"package-name": "headasbooleantrue",
"head-as-boolean": "true",
"enable-typespec-namespace": "false",
Copy link
Member

Choose a reason for hiding this comment

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

instead of adding in enable-typespec-namespace: false here, can we instead switch package-name: headasbooleantrue to namespace: headasbooleantrue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am afraid not. Since for unbranded, we always respect typespec namespace if enable-typespec-namespace is undefined, you can see the namespace of the 3 packages are all type.model.visibility which will conflict with each other if we don't set enable-typespec-namespace as false.

if (
self.code_model.namespace.count(".") >= self._namespace_from_package_name.count(".")
or self.code_model.options["enable_typespec_namespace"]
):
Copy link
Member

Choose a reason for hiding this comment

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

the idea here is to no longer get namespace from package name, the preference should be

  1. namespace in tspconfig
  2. @clientNamespace in tsp (i know there are issues differentiating this rn from the original namspace in the spec)
  3. namespace in spec

Now by default, we get package-name from whatever namespace out of the top 3 wins. Only if you want to override that namesapce, then you would use the package-name flag. We do have to see how many specs this will influence and add namespace to tspconfigs so we don't break

Copy link
Contributor Author

@msyyc msyyc Jan 22, 2025

Choose a reason for hiding this comment

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

image
It is similar with my proposal shared in teams channel. Let us discuss about it in tomorrow's scrum meeting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
emitter:client:python Issue for the Python client emitter: @typespec/http-client-python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants