-
Notifications
You must be signed in to change notification settings - Fork 25.3k
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
Fix and Edit build time OpenAPI/ra/b #33361
base: openapi-build-docs
Are you sure you want to change the base?
Fix and Edit build time OpenAPI/ra/b #33361
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@captainsafia this is just a quick edit. It likely has lots of error I'll find tomorrow. I do have a couple questions. Note this branch fixes all the build errors in your branch. This will merge into your branch so you should get credit for the commit to main.
if (Assembly.GetEntryAssembly()?.GetName().Name != "GetDocument.Insider") | ||
{ | ||
// builder.Services.AddDefaults(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
builder.Services.AddDefaults();
generates the error:
'IServiceCollection' does not contain a definition for 'AddDefaults'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is my bad...I had trouble figuring out what would be a good illustration of a setup here.
Maybe we can do something that configures an EF Core DB context?
services.AddDbContext<MyContext>(options =>
options.UseSqlServer(Configuration.GetConnectionString("DefaultConnection")));
We'd have to add a definition for MyContext
and the package references for EF
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is my bad...I had trouble figuring out what would be a good illustration of a setup here.
Maybe we can do something that configures an EF Core DB context?
services.AddDbContext<MyContext>(options => options.UseSqlServer(Configuration.GetConnectionString("DefaultConnection")));
We'd have to add a definition for
MyContext
and the package references for EF
I have:
//if (Assembly.GetEntryAssembly()?.GetName().Name != "GetDocument.Insider") | ||
//{ | ||
builder.Services.AddDbContext<ControllerApiContext>(options => | ||
options.UseSqlServer(builder.Configuration.GetConnectionString("ControllerApiContext") | ||
?? throw new InvalidOperationException("Connection string 'ControllerApiContext' not found."))); | ||
//} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see any connection string or sql info when //if (Assembly.GetEntryAssembly()?.GetName().Name != "GetDocument.Insider")
is commented out. I don't see any difference in the generated OpenAPI file with or without the build check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see any connection string or sql info when //if (Assembly.GetEntryAssembly()?.GetName().Name != "GetDocument.Insider") is commented out
I believe this is because there isn't a connection string configured in the appsettings for this project. See changes in c86bab7.
I don't see any difference in the generated OpenAPI file with or without the build check.
In this case, that is the expected behavior. 👍🏽
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@captainsafia @martincostello
Any chance you could change the OpenApiGenerateDocumentsOptions
to NOT use --
for options? I realize that's easier to parse and pass to the tool and --
is the standard for dotnet tools. The problem with --
is that you can't comment out those elements. If's frequently helpful to comment out elements. It's illegal XML and you get the following error:
Character sequence '--' is illegal inside XML comments.
aspnetcore/fundamentals/openapi/samples/9.x/BuildTime/TestScripts/combined.ps1
Outdated
Show resolved
Hide resolved
aspnetcore/fundamentals/openapi/samples/9.x/BuildTime/TestScripts/doc_name.ps1
Outdated
Show resolved
Hide resolved
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
@captainsafia can you look this over and let me know what needs to change/delete before I can S&M it into your #33359? I'll then fix the merge conflicts in your #33359. |
Branch of #33359
Merges into #33359
Fixes #33391
Internal previews