Skip to content

Add connection remove reason#513

Open
congwang09 wants to merge 4 commits intomainfrom
add-connection-remove-reason
Open

Add connection remove reason#513
congwang09 wants to merge 4 commits intomainfrom
add-connection-remove-reason

Conversation

@congwang09
Copy link
Contributor

Resolves: #399

Add a "reason" field for archived connection.
reason can be:
API: user deletes connection
Failure: connection deleted because of failure

@coveralls
Copy link

coveralls commented Jan 28, 2026

Pull Request Test Coverage Report for Build 21446560245

Details

  • 4 of 7 (57.14%) changed or added relevant lines in 2 files are covered.
  • 62 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.02%) to 53.876%

Changes Missing Coverage Covered Lines Changed/Added Lines %
sdx_controller/handlers/connection_handler.py 3 6 50.0%
Files with Coverage Reduction New Missed Lines %
sdx_controller/handlers/connection_handler.py 62 66.0%
Totals Coverage Status
Change from base Build 20488537231: -0.02%
Covered Lines: 1265
Relevant Lines: 2348

💛 - Coveralls

f"Removing connection: {service_id} {connection.get('status')}"
)
_, code = self.remove_connection(te_manager, connection["id"])
_, code = self.remove_connection(
Copy link
Contributor

Choose a reason for hiding this comment

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

@congwang09 please help me understand here: will this code remove the user request from database once there is failure? I think rather than removing, we should just make the connection as state=down, no? Sorry if I misunderstood the ideia here.

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, I think we had some discussion earlier, we would mark connection status as error and remove a connection if connection contains failed links. We can definitely discuss how we should handle this situation. This would be a relatively big change, so we can probably update this in a separate PR.

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.

Add a "reason" field for archived connection

3 participants