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

Microsoft.extensions.api description.server allow specifying environment #55836

Conversation

MattyLeslie
Copy link
Contributor

@MattyLeslie MattyLeslie commented May 22, 2024

Add Support for Specifying Environment During Swagger Generation with OpenApiGenerateEnvironment MSBuild Property

This pull request adds support for specifying the environment used during Swagger document generation in ASP.NET Core projects. By introducing the OpenApiGenerateEnvironment MSBuild property, users can control the environment configuration used when generating Swagger documentation during the build process.

Changes made:

  1. Introduced OpenApiGenerateEnvironment MSBuild Property
  2. Modified MSBuild Targets

Fixes #54698

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-commandlinetools Includes: Command line tools, dotnet-dev-certs, dotnet-user-jwts, and OpenAPI label May 22, 2024
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label May 22, 2024
@MattyLeslie MattyLeslie marked this pull request as ready for review May 22, 2024 14:25
@mkArtakMSFT mkArtakMSFT added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates and removed area-commandlinetools Includes: Command line tools, dotnet-dev-certs, dotnet-user-jwts, and OpenAPI labels May 24, 2024
@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Jun 1, 2024
@MattyLeslie
Copy link
Contributor Author

@adityamandaleeka, may I please request a review on this.

Copy link

@JoasE JoasE left a comment

Choose a reason for hiding this comment

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

I don't think this will work because this just appends the --environment parameter to the dotnet-getdocument.dll command, which doesn't support the flag. I tested by using OpenApiGenerateDocumentsOptions tag as you can see in the code above can be used to accomplish appending the --environment parameter in the same way you are trying to do.

@MattyLeslie MattyLeslie force-pushed the Microsoft.Extensions.ApiDescription.Server-Allow-specifying---environment branch from b88c6d5 to da2374d Compare July 26, 2024 07:41
@MattyLeslie
Copy link
Contributor Author

I don't think this will work because this just appends the --environment parameter to the dotnet-getdocument.dll command, which doesn't support the flag. I tested by using OpenApiGenerateDocumentsOptions tag as you can see in the code above can be used to accomplish appending the --environment parameter in the same way you are trying to do.

Thanks @JoasE, I have implemented the feedback, please let me know if that is fine. Many thanks

…t) is not empty && Quoted the $(OpenApiGenerateEnvironment) value to handle cases where it contains spaces.
@dotnet-policy-service dotnet-policy-service bot added the area-commandlinetools Includes: Command line tools, dotnet-dev-certs, dotnet-user-jwts, and OpenAPI label Aug 1, 2024
….Extensions.ApiDescription.Server.targets

Co-authored-by: Martin Costello <martin@martincostello.com>
Copy link
Member

@captainsafia captainsafia left a comment

Choose a reason for hiding this comment

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

Posted a comment inline.

Also, I think there's more work involved here than I originally outlined in the issue.

We'll have to update the list of supported options in here and pipe the environment name through.

@@ -63,6 +63,10 @@
<_DotNetGetDocumentCommand>$(_DotNetGetDocumentCommand) $(OpenApiGenerateDocumentsOptions)</_DotNetGetDocumentCommand>
</PropertyGroup>

<PropertyGroup>
Copy link
Member

Choose a reason for hiding this comment

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

I'm a little bit nervous about pulling this out into a seperate property group since they are evaluated in order of appearance. I think it would make sense to tweak this a bit and include it in reference to _DotNetGeDocumentCommand like so:

<_DotNetGetDocumentCommand Condition=" '$(OpenApiGenerateEnvironment)' != '' " >--environment &quot;$(OpenApiGenerateEnvironment)&quot;</_DotNetGetDocumentCommand>

@captainsafia captainsafia added pr: pending author input For automation. Specifically separate from Needs: Author Feedback and removed pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun labels Aug 12, 2024
@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Aug 20, 2024
@dotnet-policy-service dotnet-policy-service bot added the stale Indicates a stale issue. These issues will be closed automatically soon. label Aug 30, 2024
@MattyLeslie
Copy link
Contributor Author

Not stale

@martincostello
Copy link
Member

You need to close and re-open the PR so that the CI re-runs: #55836 (comment)

That's why the bot thinks the PR is stale.

@MattyLeslie
Copy link
Contributor Author

You need to close and re-open the PR so that the CI re-runs: #55836 (comment)

That's why the bot thinks the PR is stale.

Thanks, I will do that.

@MattyLeslie MattyLeslie closed this Sep 2, 2024
@MattyLeslie MattyLeslie reopened this Sep 2, 2024
@dotnet-policy-service dotnet-policy-service bot removed stale Indicates a stale issue. These issues will be closed automatically soon. pr: pending author input For automation. Specifically separate from Needs: Author Feedback labels Sep 2, 2024
@dotnet-policy-service dotnet-policy-service bot added this to the 10.0-preview1 milestone Sep 2, 2024
@captainsafia
Copy link
Member

@MattyLeslie Do you have any thoughts on this feedback comment?

@mkArtakMSFT mkArtakMSFT removed the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Sep 9, 2024
@captainsafia captainsafia added the Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. label Sep 10, 2024
Copy link
Contributor

Hello. I see that you've just added Needs: Author Feedback label to this PR.
That label is for Issues and not for PRs. Don't worry, I'm going to replace it with the correct one.

@dotnet-policy-service dotnet-policy-service bot added pr: pending author input For automation. Specifically separate from Needs: Author Feedback and removed Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. labels Sep 10, 2024
Copy link
Contributor

Hi @MattyLeslie.
It seems you haven't touched this PR for the last two weeks. To avoid accumulating old PRs, we're marking it as stale. As a result, it will be closed if no further activity occurs within 4 days of this comment. You can learn more about our Issue Management Policies here.

@dotnet-policy-service dotnet-policy-service bot added the stale Indicates a stale issue. These issues will be closed automatically soon. label Sep 20, 2024
@dotnet-policy-service dotnet-policy-service bot removed this from the 10.0-preview1 milestone Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-commandlinetools Includes: Command line tools, dotnet-dev-certs, dotnet-user-jwts, and OpenAPI community-contribution Indicates that the PR has been added by a community member pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun pr: pending author input For automation. Specifically separate from Needs: Author Feedback stale Indicates a stale issue. These issues will be closed automatically soon.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Microsoft.Extensions.ApiDescription.Server: Allow specifying --environment
6 participants