Skip to content
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

[Fix][cherry-pick] Handle error when switch is not connected #49

Merged
merged 9 commits into from
Dec 8, 2021

Conversation

viniarck
Copy link
Member

Fixes #26

I cherry picked commits from PR #47 to use the base branch as fix/overlapping_flows since PR #45 might get on hold depending on our priorities, and PR 47 was on top of PR 45, so this unblocks me to continue on other tasks.

Description of the change

  • Raises SwitchNotConnectedError if the switch is not connected and return 424.
  • This PR depends on this PR on kytos core, just so the connection is no longer None.

Notice that as mentioned on the dependent PR, there's still cases that we're not handling when the socket TX fails, flow_manager willl need to start listening to kytos/core.openflow.connection.error, but notice that this isn't also being propagated to via the rest request because that's sync, so for clients that need to handle that either they'll have to send an async KytosEvent for kytos.flow_manager.flows.(install|delete) when we have implemented full error handling, or they can rely on the force option that we'll have on issue #46, but that will likely take a lot longer on average - which might or not be a concern - so it'll depends on the responsiveness that the client needs.

So, now if a switch isn't connected at the time when the flow mod is about to be sent, clients will get:

{
    "code": 424,
    "description": "switch 00:00:00:00:00:00:00:01 isn't connected",
    "name": "Failed Dependency"
}

main.py Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
Co-authored-by: Antonio Francisco <[email protected]>
@viniarck viniarck requested a review from ajoaoff November 29, 2021 17:10
Copy link

@italovalcy italovalcy left a comment

Choose a reason for hiding this comment

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

The changes look good to me. Thanks for adding a handler for this error, @viniarck .

@viniarck
Copy link
Member Author

viniarck commented Dec 7, 2021

The changes look good to me. Thanks for adding a handler for this error, @viniarck .

Thanks @italovalcy and @ajoaoff for your reviews on this PR. This PR is on topf of this one PR #44, I'll merge this one here once PR 44 is reviewed again, if either of you could do a second pass on that on that'd be appreciated.

Base automatically changed from fix/overlapping_flows to master December 8, 2021 14:39
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.

FlowMod raises an Internal Server Error if switch is not connected and flows are not persisted
3 participants