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

Known* definitions mission in TS Generation #2476

Closed
sarangan12 opened this issue Apr 24, 2024 · 1 comment · Fixed by #2584
Closed

Known* definitions mission in TS Generation #2476

sarangan12 opened this issue Apr 24, 2024 · 1 comment · Fixed by #2584
Assignees
Labels

Comments

@sarangan12
Copy link
Member

For the following Typespec definition:

/** Acs Router Job Status */
union AcsRouterJobStatus {
  /** Router Job Status Pending Classification */
  "PendingClassification",
  /** Router Job Status Queued */
  "Queued",
  /** Router Job Status Assigned */
  "Assigned",
  /** Router Job Status Completed */
  "Completed",
  /** Router Job Status Closed */
  "Closed",
  /** Router Job Status Cancelled */
  "Cancelled",
  /** Router Job Status Classification Failed */
  "ClassificationFailed",
  /** Router Job Status Created */
  "Created",
  /** Router Job Status Pending Schedule */
  "PendingSchedule",
  /** Router Job Status Scheduled */
  "Scheduled",
  /** Router Job Status Schedule Failed */
  "ScheduleFailed",
  /** Router Job Status Waiting For Activation */
  "WaitingForActivation",
  string,
}

Now, If you look at the generated SDK, it looks like the following:

// @public
export type AcsRouterJobStatus = string;

But, If you look at the similar swagger generated SDK, you can find a definition as:

/** Known values of {@link AcsRouterJobStatus} that the service accepts. */
export const enum KnownAcsRouterJobStatus {
  PendingClassification = "PendingClassification",
  Queued = "Queued",
  Assigned = "Assigned",
  Completed = "Completed",
  Closed = "Closed",
  Cancelled = "Cancelled",
  ClassificationFailed = "ClassificationFailed",
  Created = "Created",
  PendingSchedule = "PendingSchedule",
  Scheduled = "Scheduled",
  ScheduleFailed = "ScheduleFailed",
  WaitingForActivation = "WaitingForActivation"
}

/**
 * Defines values for AcsRouterJobStatus. \
 * {@link KnownAcsRouterJobStatus} can be used interchangeably with AcsRouterJobStatus,
 *  this enum contains the known values that the service supports.
 * ### Known values supported by the service
 * **PendingClassification** \
 * **Queued** \
 * **Assigned** \
 * **Completed** \
 * **Closed** \
 * **Cancelled** \
 * **ClassificationFailed** \
 * **Created** \
 * **PendingSchedule** \
 * **Scheduled** \
 * **ScheduleFailed** \
 * **WaitingForActivation**
 */
export type AcsRouterJobStatus = string;

i.e You should always have a Known* Model generated. But, it is not happening on the TS Generator. I am expecting a similar definition in the Typespec generated SDK also.

@xirzec @lmazuel FYI

@MaryGao
Copy link
Member

MaryGao commented Apr 28, 2024

@sarangan12 will the event grid generated as the modular SDK? is this SDK GAed?

This is the intermedia state for extensible enum and there are two design issues in progress. And we need to support it once the designed issue is finalized.

/cc @joheredi for awareness

@qiaozha qiaozha added RLC HRLC p0 priority 0 labels May 8, 2024
sarangan12 added a commit to Azure/azure-sdk-for-js that referenced this issue Jun 7, 2024
### Packages impacted by this PR

@azure/eventgrid-system-events

### Issues associated with this PR

NA

### Describe the problem that is addressed by this PR

1. Originally, there was one unified package -
[`@Azure/eventgrid`](https://github.com/Azure/azure-sdk-for-js/tree/main/sdk/eventgrid/eventgrid).
This package was generated from Swagger specification.
2. Earlier in April 2024, some parts of the `@Azure/eventgrid` package
were extracted out to create a new package -
[`@azure/eventgrid-namespaces`](https://github.com/Azure/azure-sdk-for-js/tree/main/sdk/eventgrid/eventgrid-namespaces).
This package was generated from Typespec specification.
3. Now, with this PR, we are looking at the second split of the
`@Azure/eventgrid` package. All System Events have been extracted out of
the swagger specification to Typespec specification. This is a new
package named `@azure/eventgrid-system-events`.

### What are the possible designs available to address the problem? If
there are more than one possible design, why was the one in this PR
chosen?

1. In the Typespec core generator, We have an issue where client name
overrides are not applied to `alias`. I have created an issue
microsoft/typespec#3240 to fix this. In the
meantime, I have handled this issue in the custom code.
2. With the new generator, `Known*` models are not generated. I have
created an issue
Azure/autorest.typescript#2476 to fix this. In
the meantime, I have handled this issue in the custom code.
3. In the previous generator, we had several properties which were
overridden as mandatory parameters. With the new generator, such mass
overrides are not possible. So, we are allowing them to be optional and
close to the Wore specification. This is in parity with the other
language SDKs.

### Are there test cases added in this PR? _(If not, why?)_

No. They are all model objects. No code functionality is involved.

### Provide a list of related PRs _(if any)_

1. microsoft/typespec#3240
2. Azure/autorest.typescript#2476
3. #29245

### Command used to generate this PR:**_(Applicable only to SDK release
request PRs)_

`npx tsp compile client.tsp`

### Checklists
- [X] Added impacted package name to the issue description
- [X] Does this PR needs any fixes in the SDK Generator?** _(If so,
create an Issue in the
[Autorest/typescript](https://github.com/Azure/autorest.typescript)
repository and link it here)_
- [X] Added a changelog (if necessary)

@xirzec @joheredi @lmazuel FYI......

---------

Co-authored-by: Harsha Nalluru <sanallur@microsoft.com>
@qiaozha qiaozha self-assigned this Jun 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants