Skip to content

[domain-deletion]Introduce a new API for domain deletion #6905

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

Conversation

gazi-yestemirova
Copy link
Contributor

@gazi-yestemirova gazi-yestemirova commented May 12, 2025

Detailed Description
[In-depth description of the changes made to the schema or interfaces, specifying new fields, removed fields, or modified data structures]

  • Added new DeleteDomain API to permanently remove domain records
  • The API requires the domain to be in DEPRECATED status
  • Added validation to ensure domain has no running workflows before deletion
  • Added error handling for various failure scenarios:
    - Domain not found
    - Domain not in deprecated state
    - Domain has existing workflow history
    - Deletion operation failures
    Added unit tests covering success and failure cases

Impact Analysis

  • Backward Compatibility:
    The change is backward compatible as it only adds a new API endpoint
    Existing domain operations (Register, Update, Deprecate) remain unchanged
    No changes to existing data structures or schemas

  • Forward Compatibility:
    The change is forward compatible
    New clients can use the DeleteDomain API while old clients continue to work with existing APIs
    No breaking changes to existing interfaces or data structures

Testing Plan

  • Unit Tests: Added comprehensive unit tests in common/domain/handler_test.go & service/frontend/api/domain_handlers_test.go
  • Persistence Tests: [If the change is related to a data type which is persisted, do we have persistence tests covering the change?]
  • Integration Tests: [Do we have integration test covering the change?]
  • Compatibility Tests: [Have we done tests to test the backward and forward compatibility?]

Rollout Plan

  • What is the rollout plan? Deploy to dev & staging environments first. Deploy to prod environments in waves.
  • Does the order of deployment matter? No
  • Is it safe to rollback? Does the order of rollback matter? Safe to rollback as it's an additive change
  • Is there a kill switch to mitigate the impact immediately? No specific kill switch needed

@@ -143,9 +143,24 @@ func (wh *WorkflowHandler) UpdateDomain(
return resp, nil
}

// DeprecateDomain us used to update status of a registered domain to DEPRECATED. Once the domain is deprecated
// DeleteDomain permanently removes a domain record. This operation:
// - Requires domain to be in DEPRECATED status
Copy link
Contributor

Choose a reason for hiding this comment

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

Where does this check exists? Is there a test on this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, it was added later under service/frontend/api/domain_handlers.go DeleteDomain()

expectedError string
}{
{
name: "success - delete deprecated domain",
Copy link
Contributor

Choose a reason for hiding this comment

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

nit:
It is better to call subtests with underscope:
success_delete_deprecated_domain

This way, when a test fails, you can easily find the failed case.
When you use spaces and dashes, it will not be apparent which test case caused the failure.
And I think it will look like:
TestDeleteDomain/success_-_delete_deprecated_domain

@gazi-yestemirova gazi-yestemirova enabled auto-merge (squash) May 13, 2025 09:51
@gazi-yestemirova gazi-yestemirova merged commit f707e82 into cadence-workflow:master May 13, 2025
25 checks passed
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.

2 participants