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

Remove session caching and state #3381

Merged
merged 18 commits into from
Feb 21, 2024
Merged
39 changes: 16 additions & 23 deletions cli/azd/internal/vsrpc/aspire_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,17 @@ func (s *aspireService) GetAspireHostAsync(
return nil, err
}

session.sessionMu.Lock()
defer session.sessionMu.Unlock()

var c struct {
azdContext *azdcontext.AzdContext `container:"type"`
dotnetCli dotnet.DotNetCli `container:"type"`
}

if err := session.container.Fill(&c); err != nil {
container, err := session.newContainer()
if err != nil {
return nil, err
}

if err := container.Fill(&c); err != nil {
return nil, err
}

Expand All @@ -56,25 +58,21 @@ func (s *aspireService) GetAspireHostAsync(
projectConfig *project.ProjectConfig `container:"type"`
}

if err := session.container.Fill(&cc); err != nil {
if err := container.Fill(&cc); err != nil {
return nil, err
}

if session.appHostPath == "" {
appHost, err := appHostForProject(ctx, cc.projectConfig, c.dotnetCli)
if err != nil {
return nil, err
}

session.appHostPath = appHost.Path()
appHost, err := appHostForProject(ctx, cc.projectConfig, c.dotnetCli)
if err != nil {
return nil, err
}

hostInfo := &AspireHost{
Name: filepath.Base(filepath.Dir(session.appHostPath)),
Path: session.appHostPath,
Name: filepath.Base(filepath.Dir(appHost.Path())),
Path: appHost.Path(),
}

manifest, err := session.readManifest(ctx, session.appHostPath, c.dotnetCli)
manifest, err := session.readManifest(ctx, appHost.Path(), c.dotnetCli)
if err != nil {
return nil, fmt.Errorf("failed to load app host manifest: %w", err)
}
Expand All @@ -98,13 +96,11 @@ func (s *aspireService) GetAspireHostAsync(
}

hostInfo := &AspireHost{
Name: filepath.Base(filepath.Dir(session.appHostPath)),
Name: filepath.Base(filepath.Dir(hosts[0].Path)),
Path: hosts[0].Path,
}

session.appHostPath = hostInfo.Path

manifest, err := session.readManifest(ctx, session.appHostPath, c.dotnetCli)
manifest, err := session.readManifest(ctx, hosts[0].Path, c.dotnetCli)
if err != nil {
return nil, fmt.Errorf("failed to load app host manifest: %w", err)
}
Expand All @@ -124,14 +120,11 @@ func (s *aspireService) GetAspireHostAsync(
func (s *aspireService) RenameAspireHostAsync(
ctx context.Context, sessionId Session, newPath string, observer IObserver[ProgressMessage],
) error {
session, err := s.server.validateSession(ctx, sessionId)
_, err := s.server.validateSession(ctx, sessionId)
if err != nil {
return err
}

session.sessionMu.Lock()
defer session.sessionMu.Unlock()

// TODO(azure/azure-dev#3283): What should this do? Rewrite azure.yaml? We'll end up losing comments...
return errors.New("not implemented")
}
Expand Down
23 changes: 11 additions & 12 deletions cli/azd/internal/vsrpc/environment_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,15 @@ func (s *environmentService) GetEnvironmentsAsync(
return nil, err
}

session.sessionMu.Lock()
defer session.sessionMu.Unlock()

var c struct {
envManager environment.Manager `container:"type"`
}

if err := session.container.Fill(&c); err != nil {
container, err := session.newContainer()
if err != nil {
return nil, err
}
if err := container.Fill(&c); err != nil {
return nil, err
}

Expand Down Expand Up @@ -71,14 +72,15 @@ func (s *environmentService) SetCurrentEnvironmentAsync(
return false, err
}

session.sessionMu.Lock()
defer session.sessionMu.Unlock()

var c struct {
azdCtx *azdcontext.AzdContext `container:"type"`
}

if err := session.container.Fill(&c); err != nil {
container, err := session.newContainer()
if err != nil {
return false, err
}
if err := container.Fill(&c); err != nil {
return false, err
}

Expand All @@ -94,14 +96,11 @@ func (s *environmentService) SetCurrentEnvironmentAsync(
func (s *environmentService) DeleteEnvironmentAsync(
ctx context.Context, sessionId Session, name string, observer IObserver[ProgressMessage],
) (bool, error) {
session, err := s.server.validateSession(ctx, sessionId)
_, err := s.server.validateSession(ctx, sessionId)
if err != nil {
return false, err
}

session.sessionMu.Lock()
defer session.sessionMu.Unlock()

// TODO(azure/azure-dev#3285): Implement this.
return false, errors.New("not implemented")
}
Expand Down
35 changes: 16 additions & 19 deletions cli/azd/internal/vsrpc/environment_service_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@ func (s *environmentService) CreateEnvironmentAsync(
return false, err
}

session.sessionMu.Lock()
defer session.sessionMu.Unlock()

envSpec := environment.Spec{
Name: newEnv.Name,
Subscription: newEnv.Properties["Subscription"],
Expand All @@ -40,32 +37,32 @@ func (s *environmentService) CreateEnvironmentAsync(
envManager environment.Manager `container:"type"`
}

if err := session.container.Fill(&c); err != nil {
container, err := session.newContainer()
if err != nil {
return false, err
}
if err := container.Fill(&c); err != nil {
return false, err
}

// If an azure.yaml doesn't already exist, we need to create one. Creating an environment implies initializing the
// azd project if it does not already exist.
if _, err := os.Stat(c.azdContext.ProjectPath()); errors.Is(err, fs.ErrNotExist) {
// Write an azure.yaml file to the project.
if session.appHostPath == "" {
hosts, err := appdetect.DetectAspireHosts(ctx, c.azdContext.ProjectDirectory(), c.dotnetCli)
if err != nil {
return false, fmt.Errorf("failed to discover app host project under %s: %w", c.azdContext.ProjectPath(), err)
}

if len(hosts) == 0 {
return false, fmt.Errorf("no app host projects found under %s", c.azdContext.ProjectPath())
}
hosts, err := appdetect.DetectAspireHosts(ctx, c.azdContext.ProjectDirectory(), c.dotnetCli)
if err != nil {
return false, fmt.Errorf("failed to discover app host project under %s: %w", c.azdContext.ProjectPath(), err)
}

if len(hosts) > 1 {
return false, fmt.Errorf("multiple app host projects found under %s", c.azdContext.ProjectPath())
}
if len(hosts) == 0 {
return false, fmt.Errorf("no app host projects found under %s", c.azdContext.ProjectPath())
}

session.appHostPath = hosts[0].Path
if len(hosts) > 1 {
return false, fmt.Errorf("multiple app host projects found under %s", c.azdContext.ProjectPath())
}

manifest, err := session.readManifest(ctx, session.appHostPath, c.dotnetCli)
manifest, err := session.readManifest(ctx, hosts[0].Path, c.dotnetCli)
if err != nil {
return false, fmt.Errorf("reading app host manifest: %w", err)
}
Expand All @@ -75,7 +72,7 @@ func (s *environmentService) CreateEnvironmentAsync(
c.azdContext.ProjectDirectory(),
filepath.Base(c.azdContext.ProjectDirectory()),
manifest,
session.appHostPath)
hosts[0].Path)
if err != nil {
return false, fmt.Errorf("generating project artifacts: %w", err)
}
Expand Down
25 changes: 13 additions & 12 deletions cli/azd/internal/vsrpc/environment_service_deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,19 @@ func (s *environmentService) DeployAsync(
return nil, err
}

session.sessionMu.Lock()
defer session.sessionMu.Unlock()

outputWriter := &lineWriter{
next: &messageWriter{
ctx: ctx,
observer: observer,
},
}

session.outWriter.AddWriter(outputWriter)
defer session.outWriter.RemoveWriter(outputWriter)
container, err := session.newContainer()
if err != nil {
return nil, err
}
container.outWriter.AddWriter(outputWriter)
defer container.outWriter.RemoveWriter(outputWriter)

provisionFlags := cmd.NewProvisionFlagsFromEnvAndOptions(
&internal.EnvFlag{
Expand All @@ -57,25 +58,25 @@ func (s *environmentService) DeployAsync(
},
)

session.container.MustRegisterScoped(func() internal.EnvFlag {
container.MustRegisterScoped(func() internal.EnvFlag {
return internal.EnvFlag{
EnvironmentName: name,
}
})

ioc.RegisterInstance[*cmd.ProvisionFlags](session.container, provisionFlags)
ioc.RegisterInstance[*cmd.DeployFlags](session.container, deployFlags)
ioc.RegisterInstance[[]string](session.container, []string{})
ioc.RegisterInstance[*cmd.ProvisionFlags](container.NestedContainer, provisionFlags)
ioc.RegisterInstance[*cmd.DeployFlags](container.NestedContainer, deployFlags)
ioc.RegisterInstance[[]string](container.NestedContainer, []string{})

session.container.MustRegisterNamedTransient("provisionAction", cmd.NewProvisionAction)
session.container.MustRegisterNamedTransient("deployAction", cmd.NewDeployAction)
container.MustRegisterNamedTransient("provisionAction", cmd.NewProvisionAction)
container.MustRegisterNamedTransient("deployAction", cmd.NewDeployAction)

var c struct {
deployAction actions.Action `container:"name"`
provisionAction actions.Action `container:"name"`
}

if err := session.container.Fill(&c); err != nil {
if err := container.Fill(&c); err != nil {
return nil, err
}

Expand Down
26 changes: 10 additions & 16 deletions cli/azd/internal/vsrpc/environment_service_load.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,6 @@ func (s *environmentService) OpenEnvironmentAsync(
return nil, err
}

session.sessionMu.Lock()
defer session.sessionMu.Unlock()

return s.loadEnvironmentAsyncWithSession(ctx, session, name, false)
}

Expand All @@ -49,9 +46,6 @@ func (s *environmentService) LoadEnvironmentAsync(
return nil, err
}

session.sessionMu.Lock()
defer session.sessionMu.Unlock()

return s.loadEnvironmentAsyncWithSession(ctx, session, name, true)
}

Expand All @@ -66,7 +60,11 @@ func (s *environmentService) loadEnvironmentAsyncWithSession(
dotnetCli dotnet.DotNetCli `container:"type"`
}

if err := session.container.Fill(&c); err != nil {
container, err := session.newContainer()
if err != nil {
return nil, err
}
if err := container.Fill(&c); err != nil {
return nil, err
}

Expand Down Expand Up @@ -110,20 +108,16 @@ func (s *environmentService) loadEnvironmentAsyncWithSession(

// If we would have to discover the app host or load the manifest from disk and the caller did not request it
// skip this somewhat expensive operation, at the expense of not building out the services array.
if (session.appHostPath == "" || session.manifestCache[session.appHostPath] == nil) && !mustLoadServices {
if !mustLoadServices {
return ret, nil
}

if session.appHostPath == "" {
appHost, err := appHostForProject(ctx, c.projectConfig, c.dotnetCli)
if err != nil {
return nil, fmt.Errorf("failed to find Aspire app host: %w", err)
}

session.appHostPath = appHost.Path()
appHost, err := appHostForProject(ctx, c.projectConfig, c.dotnetCli)
if err != nil {
return nil, fmt.Errorf("failed to find Aspire app host: %w", err)
}

manifest, err := session.readManifest(ctx, session.appHostPath, c.dotnetCli)
manifest, err := session.readManifest(ctx, appHost.Path(), c.dotnetCli)
if err != nil {
return nil, fmt.Errorf("reading app host manifest: %w", err)
}
Expand Down
11 changes: 6 additions & 5 deletions cli/azd/internal/vsrpc/environment_service_refresh.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,6 @@ func (s *environmentService) RefreshEnvironmentAsync(
return nil, err
}

session.sessionMu.Lock()
defer session.sessionMu.Unlock()

return s.refreshEnvironmentAsyncWithSession(ctx, session, name, observer)
}

Expand All @@ -60,13 +57,17 @@ func (s *environmentService) refreshEnvironmentAsyncWithSession(
envManager environment.Manager `container:"type"`
}

session.container.MustRegisterScoped(func() internal.EnvFlag {
container, err := session.newContainer()
if err != nil {
return nil, err
}
container.MustRegisterScoped(func() internal.EnvFlag {
return internal.EnvFlag{
EnvironmentName: name,
}
})

if err := session.container.Fill(&c); err != nil {
if err := container.Fill(&c); err != nil {
return nil, err
}

Expand Down
11 changes: 7 additions & 4 deletions cli/azd/internal/vsrpc/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ type Server struct {
sessions map[string]*serverSession
// sessionsMu protects access to sessions.
sessionsMu sync.Mutex
// rootContainer contains all the core registrations for the azd components. Each session creates a new scope from
// this root container.
// rootContainer contains all the core registrations for the azd components.
// It is not expected to be modified throughout the lifetime of the server.
rootContainer *ioc.NestedContainer
}

Expand Down Expand Up @@ -75,7 +75,7 @@ func serveRpc(w http.ResponseWriter, r *http.Request, handlers map[string]Handle
cancelersMu := sync.Mutex{}

rpcServer.Go(r.Context(), func(ctx context.Context, reply jsonrpc2.Replier, req jsonrpc2.Request) error {
log.Printf("handling rpc for %s", req.Method())
log.Printf("handling rpc %s", req.Method())

// Observe cancellation messages from the client to us. The protocol is a message sent to the `$/cancelRequest`
// method with an `id` parameter that is the ID of the request to cancel. For each inflight RPC we track the
Expand Down Expand Up @@ -115,6 +115,7 @@ func serveRpc(w http.ResponseWriter, r *http.Request, handlers map[string]Handle
}

go func() {
start := time.Now()
var childCtx context.Context = ctx

// If this is a call, create a new context and cancel function to track the request and allow it to be
Expand Down Expand Up @@ -143,7 +144,9 @@ func serveRpc(w http.ResponseWriter, r *http.Request, handlers map[string]Handle
}

if err != nil {
log.Printf("handler for rpc %s returned error: %v", req.Method(), err)
log.Printf("handled rpc %s in %s with err: %v", req.Method(), time.Since(start), err)
} else {
log.Printf("handled rpc %s in %s", req.Method(), time.Since(start))
}
}()

Expand Down
Loading
Loading