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

specs, clientNamespace #775

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

weidongxu-microsoft
Copy link
Member

@weidongxu-microsoft weidongxu-microsoft commented Nov 20, 2024

fix #758

This scenario mainly test the case when @clientNamespace applied to TypeSpec Namespace/Interface/Model/Enum.

So far it does not test corner cases, like client/subclient in different namespace, name conflict for model in different namespace, etc.

Cadl Ranch Contribution Checklist:

  • I have written a scenario spec
  • I have meaningful @scenario names. Someone can look at the list of scenarios and understand what I'm covering.
  • I have written a mock API
  • I have used @scenarioDocs for extra scenario description and to tell people how to pass my mock api check.

Copy link

changeset-bot bot commented Nov 20, 2024

⚠️ No Changeset found

Latest commit: bcbc2c5

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

name: "ClientNamespaceFirstClient",
service: _Specs_.Azure.ClientGenerator.Core.ClientNamespace,
})
@clientNamespace("azure.clientgenerator.core.clientnamespace", "java")
Copy link
Member Author

Choose a reason for hiding this comment

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

I assume for other languages:

csharp: Azure.ClientGenerator.Core.ClientNamespace
python: azure.client_generator.core.client_namespace

Let me know what the pattern for Go/JS. I will fill in here before PR public for review.

op get(): Model.SecondClientResult;

namespace Model {
model SecondClientResult {
Copy link
Member

Choose a reason for hiding this comment

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

I think it is a little weird to put model into another namespace different with client on purpose. For python SDK, we will always put model into folder models, then current cadl-ranch will results in namespace xxx.model.models in python.

Copy link
Member Author

@weidongxu-microsoft weidongxu-microsoft Nov 20, 2024

Choose a reason for hiding this comment

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

Java is same logic, it would be in model.models in azure flavor, if written like this (for unbranded, we won't automatically add that .models).

However, there is nothing prohibit unbranded from doing this?
And the ask from OpenAI is to move some models to another SDK namespace? (I am not sure whether client goes with the models)

From current openai unbranded typespec, I didn't find a different namespace. Will double check with Srikanta on the exact use case they asks.

Copy link
Member Author

@weidongxu-microsoft weidongxu-microsoft Nov 20, 2024

Choose a reason for hiding this comment

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

PS: in client.tsp, I actually map this to same client namespace of the client.

On the other hand, I actually map the SecondClientEnumType to a sub namespace. <-- this could be the concern for you

@@clientNamespace(Second.Model, "azure.clientgenerator.core.clientnamespace.second", "java");
@@clientNamespace(Second.Model.SecondClientEnumType, "azure.clientgenerator.core.clientnamespace.second.sub", "java");

Copy link
Member Author

@weidongxu-microsoft weidongxu-microsoft Nov 21, 2024

Choose a reason for hiding this comment

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

@msyyc

The possible scenario of model/client in difference space (described by Srikanta) is that there be 2 catalogs of models

  1. model shared to multiple clients
  2. model only used in a single client

If we put clients to different namespaces, there will certainly case for "shared model", some of them would have different namespace from the client that uses them.

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.

Namespace refactoring Scenario tests
2 participants