[WIP] [Feature] impl delete execution phase

[taieeuu]

Tracking issue

Why are the changes needed?

What changes were proposed in this pull request?

How was this patch tested?

Setup process

Screenshots

Check all the applicable boxes

• I updated the documentation accordingly.
• All new and existing tests passed.
• All commits are signed-off.

Related PRs

Docs link

Summary by Bito

This PR implements a new feature for deleting execution phases in Flyte, adding functionality across client libraries, CLI interface, and remote execution components. The implementation enables phase deletion through both programmatic and command-line interfaces, with comprehensive phase validation checks. The changes include error handling for gRPC calls and consistent validation of execution phases across all interface layers.

Unit tests added: False

Estimated effort to review (1-5, lower is better): 1

[flyte-bot]

Code Review Agent Run #3b0d37

Actionable Suggestions - 4

• flytekit/clients/friendly.py - 1

  • Consider validating execution phase parameter · Line 670-672
• flytekit/clients/raw.py - 1

  • Consider adding gRPC error handling · Line 419-424
• flytekit/remote/remote.py - 1

  • Consider adding phase parameter validation · Line 2684-2690
• flytekit/clis/flyte_cli/main.py - 1

  • Consider adding phase parameter validation · Line 1179-1179

Review Details

• Files reviewed - 4 · Commit Range: 189b09e..189b09e

  • flytekit/clients/friendly.py
  • flytekit/clients/raw.py
  • flytekit/clis/flyte_cli/main.py
  • flytekit/remote/remote.py
• Files skipped - 0
• Tools

  • Whispers (Secret Scanner) - ✔︎ Successful
  • Detect-secrets (Secret Scanner) - ✔︎ Successful
  • MyPy (Static Code Analysis) - ✔︎ Successful
  • Astral Ruff (Static Code Analysis) - ✔︎ Successful

---

AI Code Review powered by

[flyte-bot]

Changelist by Bito

This pull request implements the following key changes.

Key Change	Files Impacted
New Feature - Delete Execution Phase Implementation	- friendly.py - Added delete_execution_phase method to SynchronousFlyteClient - raw.py - Implemented raw delete_execution_phase API endpoint - main.py - Added CLI command for deleting execution phase - remote.py - Added delete_execution_phase method to FlyteRemote class

[flyte-bot]

Consider validating execution phase parameter

Consider validating the phase parameter against allowed execution phases before making the delete request. This could help catch invalid phase values early.

Code suggestion

Check the AI-generated fix before applying

Suggested change

	workflow_execution_id = WorkflowExecutionIdentifier(project=project, domain=domain, name="")
	request_dict = {"id": workflow_execution_id.to_flyte_idl(), "phase": phase}
	VALID_PHASES = ["QUEUED", "RUNNING", "SUCCEEDED", "FAILED", "ABORTED", "TIMED_OUT"]
	if phase not in VALID_PHASES:
	raise ValueError(f"Invalid phase '{phase}'. Must be one of {VALID_PHASES}")
	workflow_execution_id = WorkflowExecutionIdentifier(project=project, domain=domain, name="")
	request_dict = {"id": workflow_execution_id.to_flyte_idl(), "phase": phase}

Code Review Run #3b0d37

---

Should Bito avoid suggestions like this for future reviews? (Manage Rules)

• Yes, avoid them

[flyte-bot]

Consider adding gRPC error handling

Consider adding error handling for potential gRPC exceptions in the delete_execution_phase method. The current implementation might fail silently if the gRPC call encounters network issues or server errors.

Code suggestion

Check the AI-generated fix before applying

Suggested change

	def delete_execution_phase(self, delete_execution_phase_request):
	"""
	:param flyteidl.admin.execution_pb2.DeleteExecutionPhaseRequest delete_execution_phase_request:
	:rtype: flyteidl.admin.execution_pb2.DeleteExecutionPhaseResponse
	"""
	return self._stub.DeleteExecutionPhase(delete_execution_phase_request, metadata=self._metadata)
	def delete_execution_phase(self, delete_execution_phase_request):
	"""
	:param flyteidl.admin.execution_pb2.DeleteExecutionPhaseRequest delete_execution_phase_request:
	:rtype: flyteidl.admin.execution_pb2.DeleteExecutionPhaseResponse
	"""
	try:
	return self._stub.DeleteExecutionPhase(delete_execution_phase_request, metadata=self._metadata)
	except grpc.RpcError as e:
	logger.error(f"Failed to delete execution phase: {e}")
	raise

Code Review Run #3b0d37

---

Should Bito avoid suggestions like this for future reviews? (Manage Rules)

• Yes, avoid them

[flyte-bot]

Consider adding phase parameter validation

Consider validating the phase parameter against allowed execution phases before making the delete call. This could help catch invalid phase values early.

Code suggestion

Check the AI-generated fix before applying

Suggested change

	def delete_execution_phase(self, execution: FlyteWorkflowExecution, phase: str):
	"""Delete a phase from executions.
	:param execution: workflow execution to delete phase from
	:param phase: phase to delete
	"""
	self.client.delete_execution_phase(execution.id.project, execution.id.domain, phase)
	def delete_execution_phase(self, execution: FlyteWorkflowExecution, phase: str):
	"""Delete a phase from executions.
	:param execution: workflow execution to delete phase from
	:param phase: phase to delete - must be a valid execution phase
	:raises ValueError: if phase is not a valid execution phase
	"""
	valid_phases = {"RUNNING", "SUCCEEDED", "FAILED", "ABORTED"}
	if phase not in valid_phases:
	raise ValueError(f"Invalid phase {phase}. Must be one of {valid_phases}")
	self.client.delete_execution_phase(execution.id.project, execution.id.domain, phase)

Code Review Run #3b0d37

---

Should Bito avoid suggestions like this for future reviews? (Manage Rules)

• Yes, avoid them

[flyte-bot]

Consider adding phase parameter validation

Consider adding validation for the phase parameter to ensure it matches valid execution phase values. The phase parameter appears to be used directly without validation which could lead to invalid phase deletion attempts.

Code suggestion

Check the AI-generated fix before applying

Suggested change

	def delete_execution_phase(host, insecure, project, domain, phase):
	VALID_PHASES = ["QUEUED", "RUNNING", "SUCCEEDED", "FAILED", "ABORTED", "TIMED_OUT"]
	def delete_execution_phase(host, insecure, project, domain, phase):
	if phase not in VALID_PHASES:
	click.echo(f"\nInvalid phase value: {phase}. Valid phases are: {', '.join(VALID_PHASES)}")
	sys.exit(1)

Code Review Run #3b0d37

---

Should Bito avoid suggestions like this for future reviews? (Manage Rules)

• Yes, avoid them