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

Create Fleet: panel implementation #1161

Merged
merged 4 commits into from
Jan 15, 2025
Merged

Conversation

JunyuQian
Copy link
Contributor

Description

This pull request introduces the basic panel implementation and required webview definitions for creating fleets in the VSCode extension.

Key Changes

  1. Added Fleet Management Capabilities:

    • Introduced ContainerServiceFleetClient to manage AKS fleets.
    • Implemented getAksFleetClient function in arm.ts to create a fleet client.
    • Added CreateFleetPanel and CreateFleetDataProvider classes in CreateFleetPanel.ts for handling fleet creation UI and logic.
  2. New Webview Definitions:

    • Created createFleet.ts under webviewDefinitions to define interfaces and message types related to fleet creation.
    • Updated webviewTypes.ts to include CreateFleetDefinition.
  3. UI Enhancements:

    • Added placeholder for createFleet in main.tsx and manualTest/main.tsx to ensure compilation and future implementation. The code in main.tsx is hardcoded to satisfy dependencies for the project to compile.

Changes

  • Files Modified:
    • src/commands/utils/arm.ts: Added ContainerServiceFleetClient and getAksFleetClient.
    • src/panels/CreateFleetPanel.ts: Created new panel and data provider for fleet creation.
    • src/webview-contract/webviewDefinitions/createFleet.ts: Defined types for fleet creation webview.
    • src/webview-contract/webviewTypes.ts: Added CreateFleetDefinition.
    • webview-ui/src/main.tsx: Added placeholder for fleet creation with hardcoded code to satisfy dependencies.
    • webview-ui/src/manualTest/main.tsx: Added placeholder for fleet creation test scenarios with hardcoded code to satisfy dependencies.

Testing

  • Placeholder components have been added to ensure dependencies are resolved during compilation.
  • Full implementation and test cases will be added in subsequent PRs.

Notes

  • The right-click entry point for createFleet is not yet visible to users to avoid incomplete experience.
  • The creation of resource groups is yet to be included.
  • Test cases are yet to be added.

@Tatsinnit Tatsinnit added enhancement 🚀 New feature or request or improvements on existing code. UX labels Jan 13, 2025
src/commands/utils/arm.ts Show resolved Hide resolved
src/panels/CreateFleetPanel.ts Show resolved Hide resolved
src/panels/CreateFleetPanel.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@tejhan tejhan left a comment

Choose a reason for hiding this comment

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

Setup of everything LGTM !👍, would say some descriptive comments for some functions could help (Like for the validation criteria being used). I'm assuming the ProgressEventTypes are placeholders for now & could be modified in further PR's.

src/panels/CreateFleetPanel.ts Outdated Show resolved Hide resolved
src/panels/CreateFleetPanel.ts Outdated Show resolved Hide resolved
src/panels/CreateFleetPanel.ts Outdated Show resolved Hide resolved
src/panels/CreateFleetPanel.ts Outdated Show resolved Hide resolved
Tatsinnit
Tatsinnit previously approved these changes Jan 14, 2025
Copy link
Member

@Tatsinnit Tatsinnit left a comment

Choose a reason for hiding this comment

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

🐲 Thank you for this PR—this second part of the web view looks good to me! While it’s not yet connected, it’s clear that it’s heading in the right direction, and the U/X folks seem happy with it as well.

We’re now entering a crucial stage beyond this point, where connecting everything together will become key. I’d strongly encourage you to engage with Tejhan and Reinier pro-actively with the next PR, as it will likely be even more critical in tying everything together and ensuring success.

Once again, thank you for your efforts on this PR—it’s an important step forward. Feel free to reach out if there’s anything I can help with, or if more collaboration is needed as we move toward the next phase!

Humble thanks to everyone above for valuable comments.

  • Once you cater left over comments, we can plan to squash, merge this mid-day today. 🤞 thanks.

@ReinierCC
Copy link
Collaborator

Looks good! Solid foundation so far.

As you start connecting things the next tricky part will be setting up the "state.ts" file which can be thought as the message/event handling on the webview side.

I'd recommend you try to load a simple "Hello World" before tackling the full fleet webview.

Feel free to reach out with any questions!

ReinierCC
ReinierCC previously approved these changes Jan 14, 2025
@JunyuQian JunyuQian dismissed stale reviews from ReinierCC and Tatsinnit via 6830d47 January 14, 2025 23:19
@JunyuQian
Copy link
Contributor Author

Thank you all for your valuable comments!! I have carefully reviewed them and addressed all the issues in the latest commit :)

@Tatsinnit Tatsinnit merged commit c8788ff into Azure:main Jan 15, 2025
9 checks passed
@JunyuQian JunyuQian deleted the create-fleet branch January 15, 2025 02:27
@Tatsinnit Tatsinnit mentioned this pull request Feb 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 🚀 New feature or request or improvements on existing code. UX
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants