-
Notifications
You must be signed in to change notification settings - Fork 32
feat: clear destination for iceberg/s3 #238
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: refactor/worker
Are you sure you want to change the base?
Conversation
…e-ui into feat/clear_destination
server/internal/dto/response.go
Outdated
| type ClearDestinationStatusResposne struct { | ||
| Running bool `json:"running"` | ||
| } |
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.
shouldn't this status as key and value running, failed, completed etc
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.
Removed status endpoint now. Using the response from JobTask API
| } | ||
|
|
||
| type StreamDifferenceResponse struct { | ||
| DifferenceStreams map[string]interface{} `json:"difference_streams"` |
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.
it should be string , similar to streams
| LastRunType string `json:"last_run_type,omitempty"` // "sync" | "clear-destination" | ||
| CreatedAt string `json:"created_at"` | ||
| UpdatedAt string `json:"updated_at"` | ||
| Activate bool `json:"activate"` | ||
| CreatedBy string `json:"created_by,omitempty"` | ||
| UpdatedBy string `json:"updated_by,omitempty"` | ||
| } | ||
|
|
||
| type JobTask struct { | ||
| Runtime string `json:"runtime"` | ||
| StartTime string `json:"start_time"` | ||
| Status string `json:"status"` | ||
| FilePath string `json:"file_path"` | ||
| JobType string `json:"job_type"` // "sync" | "clear-destination" |
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.
why different fields for job sync or clear destination
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.
We need to show in the JobTasks API if the last run is sync or clear
| projectID := c.Ctx.Input.Param(":projectid") | ||
| id := GetIDFromPath(&c.Controller) | ||
| resp, err := c.jobService.CancelJobRun(c.Ctx.Request.Context(), projectID, id) | ||
| resp, err := c.jobService.CancelJobRun(projectID, id) |
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.
why?
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.
Context is not being used by that service, that's why I have removed
server/internal/handlers/job.go
Outdated
| // @router /project/:projectid/jobs/:id/clear-destination [get] | ||
| func (c *JobHandler) GetClearDestinationStatus() { |
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.
why different apis, job task api wouldn't be enough to tell status of last running workflow for that particular job?
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.
Yes, removed
| } | ||
|
|
||
| // start clear destination | ||
| var diffCatalog map[string]interface{} |
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.
user can update the running frequency of the job as well do we need to clear in that case as well, and can add new streams as well
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.
No, in case of frequency, we will get empty diff_streams so clear will not run
| running, err := isClearRunning(ctx, s.tempClient, projectID, jobID) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("Failed to get clear destination status: %s", err) | ||
| } | ||
| if running { | ||
| return nil, constants.ErrInProgress | ||
| } | ||
|
|
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.
instead of checking here can we create clear activity and sync activity and execuete based on isClear flag in sync api
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.
Discussed already. We need a new endpoint
* feat: show error logs in test connection in source and destination * fix: fix test connection fail modal * fix: resolve comments * chore: add comments (#224) * chore: add comments * chore: 🚨 lint fix * feat: add error logs in check connection for source and destination (#195) * feat: add error logs i check conenction for source and destination * fix: parse json message as string * chore: update response key * chore: refactor * chore: remove logEntry struct * fix: log parsing * fix: log parsing * fix: logs parsing logic * docs: updated api contract * test: ✅ base setup for automation testing using play… (#203) * test: ✅ base setup for automation testing using playwright , add test cases fro major flows * fix: integration workflow changes * fix: ci changes * fix: ci changes * fix: ci changes * fix: ci changes * fix: ci changes * fix: ci changes * fix: ci changes * fix: ci changes * fix: ci changes * fix: ci changes * fix: ci changes * fix: ci changes * fix: ci changes * fix: ci changes * fix: ci changes * fix: ci changes * test: add postgres flow * fix: lint changes * test: add iceberg specific test * fix: tests * fix: stream name * fix: test fix * fix: test fix * fix: test fix * fix: test fix * fix: changes * fix: ci changes * fix: ci changes * fix: ci changes * test: fail test on test connection failure * fix: ci changes * fix: changes * fix: changes * fix: changes * fix: changes * fix: changes * fix: changes * fix: test fix * fix: changes * fix: changes * fix: test-data fix * fix: test-data fix * fix: changes * fix: changes * fix: changes * fix: host and port changes * fix: changes * fix: change * test: login test updated * test: refactor create source and destination page * test: refactor tests * fix: test fix * fix: changes * fix: fix changes * fix: changes * fix: changes * fix: review changes * refactor: refactor tests to use authenticated state of playwright * fix: changes * docs: update tests readme * test: intercept and log destination spec response * docs: update readme * fix: update timeouts for playwright tests * test: update readme * fix: add retries in playwright config * test: fix timeout duration * fix: update readme * test: add connector data test-id * fix: resolve comments * refactor(test): make and use enums instead of strings * fix: fix test data builder * fix: test data job name * fix: add todo --------- Co-authored-by: Duke Dhal <[email protected]> Co-authored-by: deepanshupal09-datazip <[email protected]> * feat: show refetch success message in job history page (#227) * Added success message on Job History refetch * Update ui/src/modules/jobs/pages/JobHistory.tsx Co-authored-by: deepanshupal09-datazip <[email protected]> --------- Co-authored-by: vikash choudhary <[email protected]> Co-authored-by: deepanshupal09-datazip <[email protected]> * fix: replace deprecated Phosphor icons with latest *Icon alternatives (#216) * fix: replace deprecated Phosphor icons with latest *Icon alternatives * fix: update Phosphor icons to latest alternatives in DestinationEdit and SourceEdit components and fix Lint issue * refactor: remove EditSourceModal * fix: update icon import --------- Co-authored-by: vikash choudhary <[email protected]> * feat: add clear destination * fix: merge conflicts * fix: build error fix * feat: append only mode (#232) * feat: add append only mode in ui * fix: data filter bug * fix: fix color of append only mode switch * fix: add custom option in all streams ingestion mode change * fix: use clsx for conditional styling * fix: fix icons * chore: add utils --------- Co-authored-by: Taraka Swathi <[email protected]> Co-authored-by: Ankit Sharma <[email protected]> Co-authored-by: vikash choudhary <[email protected]> Co-authored-by: Duke Dhal <[email protected]> Co-authored-by: SARTHAK RANA <[email protected]> Co-authored-by: vishalm0509 <[email protected]>
|
Created a New PR for this feature: #243 |
Description
This PR introduces the Clear Destination feature that allows users to manually or automatically clear data from a destination linked to a job when configuration or stream changes are detected.
Added two new API endpoints:
POST
/api/v1/project/:projectid/jobs/:id/clear-destination– Triggers a clear destination workflow for the specified job.GET
/api/v1/project/:projectid/jobs/:id/clear-destination– Returns the current status of the clear destination workflow (running or not running).Type of change
How Has This Been Tested?