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

Hot Reload the OpenApi Document #1870

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

Conversation

aaronburtle
Copy link
Contributor

Why make this change?

Part of the ongoing work for #67

this closes #1869

This change will reload the open api docs during a hot reload scenario.

What is this change?

Update the documentor class to get a new runtimeconfig rather than storing one internally. Then add a documentor to the runtimconfig provider, and pass it into the config file watcher. We then can call the create document function when we are hot reloading.

How was this tested?

Testing is still needed. Will keep as draft until tested.

Sample Request(s)

  • Example REST and/or GraphQL request to demonstrate modifications
  • Example of CLI usage to demonstrate modifications

@ayush3797
Copy link
Contributor

This will also hot reload only the runtimeconfig section in the openapi doc? If rest.enabled changes from true to false, will the openapi doc no longer be available?

@@ -0,0 +1,69 @@
// Copyright (c) Microsoft Corporation.
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this class being shown as newly added, this was already a part of your last PR that got merged.

@@ -79,10 +79,10 @@ public void ConfigureServices(IServiceCollection services)
null);
IFileSystem fileSystem = new FileSystem();
FileSystemRuntimeConfigLoader configLoader = new(fileSystem, configFileName, connectionString);
services.AddSingleton<IOpenApiDocumentor, OpenApiDocumentor>();
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a check to make sure we do this only when Rest is enabled. ref: https://github.com/Azure/data-api-builder/pull/1872/files

response = await client.SendAsync(request);
Assert.IsTrue(response.StatusCode is HttpStatusCode.OK);
// overwrites the changes we made when hot reloading
fileSystem.File.WriteAllText(configFilePath, config.ToJson());
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the use of this last line in testing?

@@ -166,8 +167,9 @@ public void CreateDocument()
private OpenApiPaths BuildPaths()
{
OpenApiPaths pathsCollection = new();
RuntimeConfig config = _configProvider.GetConfig();
Copy link
Contributor

Choose a reason for hiding this comment

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

should we pass the config here from CreateDocument() ? is there a possibility the config that is sourced in CreateDocument() is different than the config sourced in BuildPaths()?

@seantleonard seantleonard added this to the 0.12rc milestone Feb 9, 2024
@seantleonard seantleonard modified the milestones: 0.12rc, 1.2rc Mar 12, 2024
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.

Hot Reload of the OpenApi Document
4 participants