Skip to content

refactor(amazonq): reorg amazonq servers by their function#857

Merged
wzxxing merged 1 commit intoaws:mainfrom
wzxxing:reorg-amazonq-server
Apr 1, 2025
Merged

refactor(amazonq): reorg amazonq servers by their function#857
wzxxing merged 1 commit intoaws:mainfrom
wzxxing:reorg-amazonq-server

Conversation

@wzxxing
Copy link
Contributor

@wzxxing wzxxing commented Mar 21, 2025

  • refactor(amazonq): move everything non-server out of language-server

Problem

AmazonQ server is contributed by many teams and there are utilities function scattered all over the language-server directory.

We need to split aws-lsp-codewhisperer project into separate projects within language-servers monorepo.

There will be further discussion about whether separate project is worthy of the efforts. But we agreed that at least splitting servers using the directory is a good start.

Solution

There will be further discussion about whether separate project is worthy of the efforts. But we agreed that at least splitting servers using the directory is a good start. A new directory shared is created to host the code that can be shared among different servers. Otherwise, each server should be restricted in their respective directories.

Tested by launching the codewhisperer server in the VSCode debug profile.

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@wzxxing wzxxing changed the title reorg amazonq server refactor(amazonq): reorg amazonq servers by their function Mar 21, 2025
@wzxxing wzxxing marked this pull request as ready for review March 21, 2025 15:36
@wzxxing wzxxing requested a review from a team as a code owner March 21, 2025 15:36
@wzxxing wzxxing assigned volodkevych and unassigned volodkevych Mar 21, 2025
@jpinkney-aws
Copy link
Contributor

Just trying to align myself with partner team expectations in the codebase, but correct me if i'm wrong but after this change:

  • utilities specific for codewhisperer should live in aws-lsp-codewhisperer
    • general utilities live in aws-lsp-core
  • aws-lsp-codewhisperer will continue to have chat + inline
  • any new agents we are porting next week should live in aws-lsp-codewhisperer
    • probably in something like aws-lsp-codewhisperer/chat/agents ?

@volodkevych
Copy link
Contributor

Let's name the folder "shared" or "common".

@volodkevych
Copy link
Contributor

volodkevych commented Mar 21, 2025

Just trying to align myself with partner team expectations in the codebase, but correct me if i'm wrong but after this change:

  • utilities specific for codewhisperer should live in aws-lsp-codewhisperer

    • general utilities live in aws-lsp-core
  • aws-lsp-codewhisperer will continue to have chat + inline

  • any new agents we are porting next week should live in aws-lsp-codewhisperer

    • probably in something like aws-lsp-codewhisperer/chat/agents ?

This is just the first step. At the end we'll get "common" folder + folder per LSP server. Agents should be a folder per agent - on the same level as other servers.

We should add detailed instructions into README.

@wzxxing wzxxing enabled auto-merge (squash) March 24, 2025 08:49
@wzxxing
Copy link
Contributor Author

wzxxing commented Mar 24, 2025

Let's name the folder "shared" or "common".

@volodkevych I have renamed the folder as shared

@wzxxing
Copy link
Contributor Author

wzxxing commented Mar 24, 2025

@jpinkney-aws Thanks for taking a look!

Just trying to align myself with partner team expectations in the codebase, but correct me if i'm wrong but after this change:

  • utilities specific for codewhisperer should live in aws-lsp-codewhisperer

    • general utilities live in aws-lsp-core

Yes

  • aws-lsp-codewhisperer will continue to have chat + inline

  • any new agents we are porting next week should live in aws-lsp-codewhisperer

    • probably in something like aws-lsp-codewhisperer/chat/agents ?

language-server directory is where each server that implements the Flare protocol lives. Where new agents lives will depend on which server the agent will be integrated into. I think chat/ makes sense if the agent will be in the chat server.

After this change the aws-lsp-codewhisperer directory will look like

server/aws-lsp-codewhisperer
├── config
├── script
└── src
    ├── client
    │   ├── sigv4
    │   ├── streamingClient
    │   └── token
    ├── language-server
    │   ├── chat
    │   ├── configuration
    │   ├── netTransform
    │   └── securityScan
    └── shared
        ├── amazonQServiceManager
        ├── auto-trigger
        ├── models
        ├── session
        ├── supplementalContextUtil
        └── telemetry

Copy link
Contributor

Choose a reason for hiding this comment

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

This should be moved to shared utils: at least getUserAgent is used in more places than telemetry

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mind if I just move the files in this change? Later on, we can decide which file to place this function.

Want to minimize the changes due to the folder structure reorganization.

Copy link
Contributor

Choose a reason for hiding this comment

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

I do not think base service should be in the client folder.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also moved it to shared

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we move it to shared? I think we should have only a folder per server inside language-server.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's not move it for now - we have code that import this file from specific path iirc, moving it will break it. Ideally we get rid of it completely, and export server factory functions directly. Can we do it as a follow-up?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we change the code that imports this file from specific path? Where is this code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will do it as follow up.

@volodkevych
Copy link
Contributor

As part of this or next PR for folder structure refactoring, can we add section into README with detailed guidance about this folder structure and where to add new code?

@volodkevych volodkevych self-requested a review March 25, 2025 10:01
Created a new directory called utilities under src/ for team to share code.
@wzxxing wzxxing requested a review from viktorsaws April 1, 2025 13:58
@wzxxing wzxxing merged commit 84d462b into aws:main Apr 1, 2025
6 checks passed
@wzxxing wzxxing deleted the reorg-amazonq-server branch April 1, 2025 17:59
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.

4 participants