Workspace and Workmachine Api Implementation #393
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's Guide by SourceryThis pull request introduces the Workspace and Workmachine APIs, including GraphQL resolvers, schema definitions, and domain logic. It enables CRUD operations and data retrieval for Workspace and Workmachine entities. Sequence diagram for creating a WorkspacesequenceDiagram
participant Client
participant GraphQL Resolver
participant Domain
participant Workspace Repo
Client->>GraphQL Resolver: infra_createWorkspace(workspace: Workspace)
GraphQL Resolver->>Domain: CreateWorkspace(ctx, workspace)
Domain->>Workspace Repo: Create(ctx, workspace)
Workspace Repo-->>Domain: Workspace
Domain-->>GraphQL Resolver: Workspace
GraphQL Resolver-->>Client: Workspace
Sequence diagram for updating a WorkspacesequenceDiagram
participant Client
participant GraphQL Resolver
participant Domain
participant Workspace Repo
Client->>GraphQL Resolver: infra_updateWorkspace(workspace: Workspace)
GraphQL Resolver->>Domain: UpdateWorkspace(ctx, workspace)
Domain->>Workspace Repo: Patch(ctx, filter, patchForUpdate)
Workspace Repo-->>Domain: Workspace
Domain-->>GraphQL Resolver: Workspace
GraphQL Resolver-->>Client: Workspace
Sequence diagram for deleting a WorkspacesequenceDiagram
participant Client
participant GraphQL Resolver
participant Domain
participant Workspace Repo
Client->>GraphQL Resolver: infra_deleteWorkspace(name: string)
GraphQL Resolver->>Domain: DeleteWorkspace(ctx, name)
Domain->>Workspace Repo: DeleteOne(ctx, filter)
Workspace Repo-->>Domain: error
Domain-->>GraphQL Resolver: error
GraphQL Resolver-->>Client: bool, error
Sequence diagram for getting a WorkspacesequenceDiagram
participant Client
participant GraphQL Resolver
participant Domain
participant Workspace Repo
Client->>GraphQL Resolver: infra_getWorkspace(name: string)
GraphQL Resolver->>Domain: GetWorkspace(ctx, name)
Domain->>Workspace Repo: FindOne(ctx, filter)
Workspace Repo-->>Domain: Workspace
Domain-->>GraphQL Resolver: Workspace
GraphQL Resolver-->>Client: Workspace
Sequence diagram for upserting a WorkmachinesequenceDiagram
participant Client
participant GraphQL Resolver
participant Domain
participant Workmachine Repo
Client->>GraphQL Resolver: infra_upsertWorkMachine(workmachine: Workmachine)
GraphQL Resolver->>Domain: UpsertWorkMachine(ctx, workmachine)
Domain->>Workmachine Repo: Create(ctx, workmachine)
Workmachine Repo-->>Domain: Workmachine
Domain-->>GraphQL Resolver: Workmachine
GraphQL Resolver-->>Client: Workmachine
Sequence diagram for updating a WorkmachinesequenceDiagram
participant Client
participant GraphQL Resolver
participant Domain
participant Workmachine Repo
Client->>GraphQL Resolver: infra_updateWorkMachine(workmachine: Workmachine)
GraphQL Resolver->>Domain: UpdateWorkMachine(ctx, workmachine)
Domain->>Workmachine Repo: Patch(ctx, filter, patchForUpdate)
Workmachine Repo-->>Domain: Workmachine
Domain-->>GraphQL Resolver: Workmachine
GraphQL Resolver-->>Client: Workmachine
Sequence diagram for updating a Workmachine statussequenceDiagram
participant Client
participant GraphQL Resolver
participant Domain
participant Workmachine Repo
Client->>GraphQL Resolver: infra_updateWorkMachineStatus(status: bool, name: string)
GraphQL Resolver->>Domain: UpdateWorkmachineStatus(ctx, status, name)
Domain->>Workmachine Repo: Patch(ctx, filter, patchForUpdate)
Workmachine Repo-->>Domain: Workmachine
Domain-->>GraphQL Resolver: bool, error
GraphQL Resolver-->>Client: bool, error
Sequence diagram for getting a WorkmachinesequenceDiagram
participant Client
participant GraphQL Resolver
participant Domain
participant Workmachine Repo
Client->>GraphQL Resolver: infra_getWorkmachine(name: string)
GraphQL Resolver->>Domain: GetWorkmachine(ctx, name)
Domain->>Workmachine Repo: FindOne(ctx, filter)
Workmachine Repo-->>Domain: Workmachine
Domain-->>GraphQL Resolver: Workmachine
GraphQL Resolver-->>Client: Workmachine
Class diagram for Workspace entityclassDiagram
class Workspace {
Name: string
AccountName: string
}
Workspace : - BaseEntity
Workspace : - ResourceMetadata
Class diagram for Workmachine entityclassDiagram
class Workmachine {
Name: string
AccountName: string
MachineSize: string
AuthorizedKeys: string
MachineStatus: bool
}
Workmachine : - BaseEntity
Workmachine : - ResourceMetadata
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @nxtcoder19 - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a description to the GraphQL schema for each field to improve discoverability and understanding.
- It would be helpful to add some examples in the workspace.yaml and workmachine.yaml files.
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| @@ -301,6 +302,65 @@ func (r *mutationResolver) InfraDeletePv(ctx context.Context, clusterName string | |||
| return true, nil | |||
| } | |||
|
|
|||
| // InfraCreateWorkspace is the resolver for the infra_createWorkspace field. | |||
There was a problem hiding this comment.
suggestion: Refactor repeated context conversion.
The pattern to extract 'ictx, err := toInfraContext(ctx)' is repeated in multiple resolver functions. Consider extracting this logic into a shared helper or middleware to reduce duplication and enhance consistency.
Suggested implementation:
"encoding/base64"
"fmt"
// You may need to add any necessary import for the infra context type if not already imported.+// InfraCreateWorkspace is the resolver for the infra_createWorkspace field.
+func (r *mutationResolver) InfraCreateWorkspace(ctx context.Context, workspace entities.Workspace) (*entities.Workspace, error) {
+ ictx, err := r.getInfraContext(ctx)
+ if err != nil {
+ return nil, err
+ }
+ return r.Domain.CreateWorkspace(ictx, workspace)
+}+// InfraUpdateWorkspace is the resolver for the infra_updateWorkspace field.
+func (r *mutationResolver) InfraUpdateWorkspace(ctx context.Context, workspace entities.Workspace) (*entities.Workspace, error) {
+ ictx, err := r.getInfraContext(ctx)
+ if err != nil {
+ return nil, err
+ }
+ // ... additional logic ...
+}+// getInfraContext extracts the infra-specific context and wraps any conversion errors.
+Adjust the return type of getInfraContext (currently declared as interface{}) to the appropriate infra context type used throughout your codebase if available.
Comment on lines
108
to
115
| fmt.Println("stream....", args.Stream) | ||
| s, err := jc.Jetstream.Stream(ctx, args.Stream) | ||
| if err != nil { | ||
| return nil, errors.NewE(err) | ||
| } | ||
|
|
||
| fmt.Println("stream....", err) | ||
| c, err := s.CreateOrUpdateConsumer(ctx, jetstream.ConsumerConfig(args.ConsumerConfig)) | ||
| fmt.Println("logge....", err) |
There was a problem hiding this comment.
suggestion: Remove or replace debug print statements.
The use of fmt.Println for logging debug information could leak unnecessary details in production. Consider using a structured logging package or remove these prints if they were temporary for debugging.
Suggested implementation:
| return wm, nil | ||
| } | ||
|
|
||
| func (d *domain) UpsertWorkMachine(ctx InfraContext, workmachine entities.Workmachine) (*entities.Workmachine, error) { |
There was a problem hiding this comment.
question (bug_risk): Clarify upsert behavior for workmachine.
The current UpsertWorkMachine implementation always calls Create without checking for an existing record. If the intent is to perform a true upsert, consider checking for existence and then updating as necessary, or use a repository method that supports upsert semantics.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Implement Workspace and Workmachine API functionality in the infrastructure service, adding support for creating, updating, deleting, and listing workspaces and workmachines
New Features:
Enhancements: