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

[tcgc] Add @apiVersion scenarios and mock apis #2087

Merged
merged 8 commits into from
Jan 27, 2025

Conversation

catalinaperalta
Copy link
Member

@catalinaperalta catalinaperalta commented Jan 16, 2025

Add scenarios and mock apis for the tcgc @apiVersion decorator

@azure-sdk
Copy link
Collaborator

azure-sdk commented Jan 16, 2025

All changed packages have been documented.

  • @azure-tools/azure-http-specs
Show changes

@azure-tools/azure-http-specs - fix ✏️

Add @apiversion scenarios and mock apis

@azure-sdk
Copy link
Collaborator

azure-sdk commented Jan 16, 2025

You can try these changes here

🛝 Playground 🌐 Website 📚 Next docs

Copy link
Member

@tadelesh tadelesh left a comment

Choose a reason for hiding this comment

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

i think the intension for this pr is to test the new @apiVersion decorator. but i did not see it in the spec. am i miss anything?

@catalinaperalta
Copy link
Member Author

i think the intension for this pr is to test the new @apiVersion decorator. but i did not see it in the spec. am i miss anything?

I only noticed that one of the examples didnt have the apiVersion decorator over it, but it's fixed now

@weidongxu-microsoft
Copy link
Member

weidongxu-microsoft commented Jan 23, 2025

  1. the @scenarioService already do @server and @service
  2. the @scenarioService must be on the namespace that contains operations (e.g. in your case, the namespace in "main.tsp")
  3. generally the TypeSpec namespace be different in different main.tsp (as emitter typically use the TypeSpec namespace for SDK package), so we'd like to have different suffix like .Header, .Path in different main.tsp.

Also, I kind of think the origin folder is better, e.g. "azure/client-generator-core/api-version/header` etc.

If I write the test, it would be

azure/client-generator-core/api-version/header/main.tsp (@scenarioService in this tsp)

/**
 * DO NOT GENERATE FROM THIS FILE USE client.tsp
 * This is just to simulate a service entrypoint
 */
import "./service.tsp";

using TypeSpec.Http;
using TypeSpec.Versioning;
using global.Azure.ClientGenerator.Core;
using Spector;

@scenarioService(
  "/azure/client-generator-core/api-version/header",
  {
    versioned: ApiVersions,
  }
)
namespace _Specs_.Azure.ClientGenerator.Core.ApiVersion.Header;

model HeaderApiVersionParam {
  @header("x-ms-version")
  version: string;
}

@scenario
@scenarioDoc("Set a header as the  service api version")
@doc("Header api version parameter.")
@route("/header")
@post
op headerApiVersion(...HeaderApiVersionParam): OkResponse;

azure/client-generator-core/api-version/service.tsp

import "@typespec/http";
import "@typespec/versioning";
import "@azure-tools/typespec-azure-core";
import "@typespec/spector";
import "@azure-tools/typespec-client-generator-core";

using TypeSpec.Http;
using TypeSpec.Versioning;
using Azure.ClientGenerator.Core;
using Spector;

@doc("Supported api versions.")
enum ApiVersions {
  @doc("Api version 2025-01-01.")
  @useDependency(Azure.Core.Versions.v1_0_Preview_2)
  v2025_01_01: "2025-01-01",
}

There is now not much in service.tsp, so it may be directly inserted into e.g. "header.tsp"

@catalinaperalta
Copy link
Member Author

catalinaperalta commented Jan 24, 2025

  1. the @scenarioService already do @server and @service
  2. the @scenarioService must be on the namespace that contains operations (e.g. in your case, the namespace in "main.tsp")
  3. generally the TypeSpec namespace be different in different main.tsp (as emitter typically use the TypeSpec namespace for SDK package), so we'd like to have different suffix like .Header, .Path in different main.tsp.

Also, I kind of think the origin folder is better, e.g. "azure/client-generator-core/api-version/header` etc.

If I write the test, it would be

azure/client-generator-core/api-version/header/main.tsp (@scenarioService in this tsp)

/**
 * DO NOT GENERATE FROM THIS FILE USE client.tsp
 * This is just to simulate a service entrypoint
 */
import "./service.tsp";

using TypeSpec.Http;
using TypeSpec.Versioning;
using global.Azure.ClientGenerator.Core;
using Spector;

@scenarioService(
  "/azure/client-generator-core/api-version/header",
  {
    versioned: ApiVersions,
  }
)
namespace _Specs_.Azure.ClientGenerator.Core.ApiVersion.Header;

model HeaderApiVersionParam {
  @header("x-ms-version")
  version: string;
}

@scenario
@scenarioDoc("Set a header as the  service api version")
@doc("Header api version parameter.")
@route("/header")
@post
op headerApiVersion(...HeaderApiVersionParam): OkResponse;

azure/client-generator-core/api-version/service.tsp

import "@typespec/http";
import "@typespec/versioning";
import "@azure-tools/typespec-azure-core";
import "@typespec/spector";
import "@azure-tools/typespec-client-generator-core";

using TypeSpec.Http;
using TypeSpec.Versioning;
using Azure.ClientGenerator.Core;
using Spector;

@doc("Supported api versions.")
enum ApiVersions {
  @doc("Api version 2025-01-01.")
  @useDependency(Azure.Core.Versions.v1_0_Preview_2)
  v2025_01_01: "2025-01-01",
}

There is now not much in service.tsp, so it may be directly inserted into e.g. "header.tsp"

Done. It would be great if we could have the docs mention this info + the conventions we'd like to follow. For example, not needing to specify @server and @service. I noticed some of the existing scenarios included that so I had added it here as well. cc @sarangan12

Copy link
Member

@weidongxu-microsoft weidongxu-microsoft left a comment

Choose a reason for hiding this comment

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

Please run pnpm generate-scenarios-summary before merge.
@sarangan12 The CI seems still not fixed (about the verification of summary matches the lib)

@catalinaperalta catalinaperalta added this pull request to the merge queue Jan 27, 2025
Merged via the queue into Azure:main with commit 64792fd Jan 27, 2025
23 checks passed
@catalinaperalta catalinaperalta deleted the api-version-scenarios branch January 27, 2025 23:15
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.

5 participants