-
Notifications
You must be signed in to change notification settings - Fork 81
Initial commit for task end points #691
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
base: master
Are you sure you want to change the base?
Conversation
Fresh avocado detected! Welcome and thank you for your contribution @bluepal-prasanthi-moparthi. My avocado-loving overlords have decreed a signed CLA is required for PRs. Please see https://github.com/arangodb/arangodb/blob/devel/CONTRIBUTING.md file to learn more or ask cla(at)arangodb.com if you have issues. |
please rerun test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces the initial implementation of task management endpoints and their corresponding tests.
- Added Task interface methods and TaskOptions in the public API.
- Implemented clientTask with Create, Read, List, Remove, and CreateWithID operations.
- Wrote end-to-end tests covering basic task creation, retrieval, listing, and cleanup.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
File | Description |
---|---|
v2/arangodb/tasks.go | Defines Task and ClientTasks interface and options |
v2/arangodb/tasks_impl.go | Implements Task CRUD operations in clientTask |
v2/arangodb/client_impl.go | Registers the new clientTask on the client |
v2/arangodb/client.go | Exposes ClientTasks on the Client interface |
v2/tests/tasks_test.go | Adds tests for creating, listing, retrieving, and removing tasks |
Comments suppressed due to low confidence (2)
v2/tests/tasks_test.go:72
- [nitpick] The log message says "Number of tasks" but prints a task ID. Update the message or format specifier to reflect the actual value being logged.
t.Logf("Number of tasks: %s\n", tasks[0].ID())
v2/tests/tasks_test.go:32
- Consider adding a test case for the
Params
method on the Task interface to ensure correct unmarshalling of task parameters.
type TaskParams struct {
v2/arangodb/tasks_impl.go
Outdated
case http.StatusOK: | ||
result := make([]Task, len(response)) | ||
for i, task := range response { | ||
fmt.Printf("Task %d: %+v\n", i, task) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove or replace this debug print statement with a structured logging call or eliminate it in production code.
fmt.Printf("Task %d: %+v\n", i, task) | |
log.Printf("Task %d: %+v", i, task) |
Copilot uses AI. Check for mistakes.
v2/arangodb/tasks_impl.go
Outdated
func (c clientTask) CreateTaskWithID(ctx context.Context, id string, options *TaskOptions) (Task, error) { | ||
// Check if task already exists | ||
existingTask, err := c.Task(ctx, id) | ||
fmt.Printf("Checking existing task with ID: %s, existingTask: %v, Error:%v", id, existingTask, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This debug log should be removed or replaced with a proper logger, as it may expose internal state in production.
Copilot uses AI. Check for mistakes.
v2/tests/tasks_test.go
Outdated
}, | ||
"TestDataForCreateTask": { | ||
Name: "TestDataForCreateTask", | ||
Command: "(function() { require('@arangodb').print(Hello); })()", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The JavaScript call references an undefined variable Hello
. It should be a string, e.g., print('Hello')
, to avoid runtime errors.
Command: "(function() { require('@arangodb').print(Hello); })()", | |
Command: "(function() { require('@arangodb').print('Hello'); })()", |
Copilot uses AI. Check for mistakes.
} | ||
|
||
func (c clientTask) Tasks(ctx context.Context) ([]Task, error) { | ||
urlEndpoint := connection.NewUrl("_api", "tasks") // Note: This should include database context, see below |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The endpoint URL lacks the database path; tasks should be scoped to the database, e.g., include the database name in the URL.
Copilot uses AI. Check for mistakes.
v2/arangodb/tasks.go
Outdated
@@ -0,0 +1,84 @@ | |||
// DISCLAIMER | |||
// | |||
// # Copyright 2024 ArangoDB GmbH, Cologne, Germany |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment prefix includes an extra #
. Align with the rest of the file by removing the #
.
// # Copyright 2024 ArangoDB GmbH, Cologne, Germany | |
// Copyright 2024 ArangoDB GmbH, Cologne, Germany |
Copilot uses AI. Check for mistakes.
This PR includes the implementation and test cases for the following task-related endpoints:
List all tasks
Get a specific task
Create a new task
Create a task with a specific ID
Remove a task