Add traffic rules to library #409

herriejr · 2023-07-17T20:30:34Z

With these changes, it is possible to control the trafficrules, defined in the unifi network management.
At this moment you can enable and disable a selected traffic rule.
(pls bear with me, since this is my 1st pull request)

Kane610 · 2023-07-25T21:11:27Z

Hello! Thank you for your PR, I started doing a review but got lost in other stuff, I will get back to you shortly!

Kane610

Its a good start, but some improvements are needed and of course tests.

aiounifi/interfaces/api_handlers.py

aiounifi/controller.py

aiounifi/models/api.py

aiounifi/interfaces/trafficrules.py

aiounifi/models/api.py

aiounifi/models/trafficrule.py

herriejr · 2023-08-22T18:10:01Z

I was working on some other projects and was on holiday. I will start implementing the requested changes and start adding the tests.

Kane610 · 2023-08-22T18:37:23Z

I recommend starting with doing a rebase as I've refactored some aspects lg the library

herriejr · 2023-08-22T20:59:53Z

Ok, I started with a rebase and I'm not completely done yet (still some cleaning up to do of the code and I still need to squeeze an "update" of the traffic rules in the code since the Unifi console does not send messages about these changes).
But the rebase solved the problem for which I added the 'v2-bool' in the first place.
I think I did something wrong, since I accidentally closed this pull request 😇. I will try to do the missing modifications in the next few days and add the tests in the following weeks.
I will also create a new pull request in the next few days.

Kane610 · 2023-08-23T05:39:14Z

Take your time!

herriejr · 2023-08-26T13:21:36Z

I made the requested modifications, and based it on the new structure.
I had to change a line in the api_handlers.py-file, since the response of this API is different (does not contain 'meta' nor 'data'-keys). I also added a few tests for the traffic rules to the tests.

Kane610

You would be helped setting up the dev env in a python3.11 venv and using the pre-commit tool. You can run the setup.sh script to do that, there are a lot of small things which would be caught by that.

aiounifi/controller.py

aiounifi/models/traffic_rule.py

aiounifi/interfaces/traffic_rules.py

aiounifi/models/traffic_rule.py

tests/conftest.py

tests/test_traffic_rules.py

Kane610 · 2023-08-26T19:38:30Z

I made the requested modifications, and based it on the new structure.
I had to change a line in the api_handlers.py-file, since the response of this API is different (does not contain 'meta' nor 'data'-keys). I also added a few tests for the traffic rules to the tests.

Yes we need to figure out a way to specify different expected data structures from different api endpoints. Maybe have some transform in the ApiRequest class

Kane610 · 2023-08-27T20:25:45Z

Good progress! I will be away a few days now so gonna be hard to get this in for 2023.9 release, but only a few things left to resolve

herriejr · 2023-08-28T20:14:05Z

Did it again, mistakenly closed the PR

Kane610

Just some small comments to move you along :)

aiounifi/interfaces/traffic_rules.py

aiounifi/interfaces/api_handlers.py

aiounifi/models/api.py

herriejr · 2023-08-30T21:37:28Z

I hope with the latest commits I am almost there

herriejr · 2023-08-31T13:13:34Z

I've made some modifications to the prepare_data method I introduced.
I've given that method some more responsibility:

parse the received response with orjson
inspect the result and depending on the structure of the result, put the data in the correct way in a TypedApiResponse-object to use throughout the library

Maybe it is more logical to place this elsewhere, but now the complexity of parsing different response structures is already concentrated in one place.
I still have to solve the problem I'm having when adding traffic_rule_payload to _mock_endpoints when running the tests; since I keep getting this error:
'ERROR tests/test_traffic_rules.py - Failed: In test_traffic_rules: function uses no argument 'traffic_rule_payload'

aiounifi/models/api.py

tests/conftest.py

Kane610 · 2023-08-31T18:26:30Z

I only had a few minutes tonight, will look more tomorrow.

Kane610

Even closer! Some things left. You shouldn't really need to change any existing test if we do this properly.

aiounifi/interfaces/api_handlers.py

aiounifi/interfaces/connectivity.py

aiounifi/interfaces/traffic_rules.py

aiounifi/models/api.py

aiounifi/models/traffic_rule.py

tests/conftest.py

Kane610

I think we're getting there! Try to take a step back and simplify what we've discussed and I think it's ready. I will need to rework a bit about where data response is parsed and error handling and stuff, it clearly doesn't work well with multiple data structures... But lets keep that out of this PR as much as possibly.

herriejr · 2023-09-07T10:52:27Z

I have not much time in the coming days, but I will take a closer look at everything this weekend.

Kane610 · 2023-09-09T07:45:06Z

I have not much time in the coming days, but I will take a closer look at everything this weekend.

Donit at your pace. I'm also away

…e aiounifi library.

Kane610 · 2023-09-27T19:11:01Z

What happened?

herriejr · 2023-09-29T21:02:20Z

Good job! You're getting it into fantastic shape

Looks like you accidentally removed the test_outlet and test_port files

I reverted those changes back, was a bit unfortunate of me.

Kane610

I like your improvements it fits well in with the overall structure.

Only more would be to handle response differences in the response rather than request class, but that's more work and let's keep out of it.

aiounifi/interfaces/api_handlers.py

aiounifi/models/api.py

aiounifi/models/traffic_rule.py

tests/conftest.py

aiounifi/models/api.py

tests/test_ports.py

tests/test_outlets.py

aiounifi/models/api.py

Remove unnecessary import Co-authored-by: Robert Svensson <Kane610@users.noreply.github.com>

…uestV2-base class.

Kane610 · 2023-10-03T21:20:20Z

I glanced from the phone and it looks really good. Will review tomorrow

Kane610

Im very happy with how this turned out. You did a great job!

Kane610 · 2023-10-05T18:20:47Z

I saw one thing that warrants a fix, test coverage

aiounifi/interfaces/traffic_rules.py 18 6 67% 24, 28, 32-37

tests/test_traffic_rules.py

…raffic rules.

Kane610

Looks good! 🎉

Anything more you want to do here or are you ready to get this merged?

herriejr · 2023-10-10T14:59:29Z

I am ready to get this merged. Thank you for your time and effort for guiding me.

Kane610 · 2023-10-10T17:01:18Z

Really good job!

ViViDboarder · 2023-10-12T03:36:09Z

I stumbled on this PR as I was looking to do something like it myself. Not only was I glad to see it was done (yesterday, no less!), I found this PR to be overall super uplifting.

@Kane610 your feedback and attitude was terrific and inspirational.

@herriejr your persistence and open mindedness to learn was impressive. Really great job!

Reading this made me feel good about open source, so I figured I’d share so you two can feel good about being a part of it. Kudos and thank you!

Kane610 · 2023-10-12T05:36:23Z

Thank you very much @ViViDboarder <3

Kane610 · 2023-10-12T20:08:54Z

@herriejr I will wait to build a release some more time, you should be perfectly fine doing a local install into the home assistant environment directly from the aiounifi clone. Ping me when you need to.

herriejr · 2023-10-19T13:55:41Z

@herriejr I will wait to build a release some more time, you should be perfectly fine doing a local install into the home assistant environment directly from the aiounifi clone. Ping me when you need to.

I've got it working locally. Gonna let it rest for a few days and then look at it again and try to clean the code up. I'll keep you posted.

Kane610 · 2023-10-24T19:13:02Z

@herriejr FYI I've published aiounifi v64 as I needed to propagate a fix to home assistant.

Kane610 · 2023-11-04T11:31:17Z

@herriejr I realised there is no tests with errors, Im reworking error handling a bit and would like to use that one better. Could you provide me with a couple of error examples for traffic rules (or rather v2 APIs)

New PR for reference #520

herriejr · 2023-11-04T15:21:57Z

I will be away for work the upcoming week. I will then look into it.

Kane610 · 2023-11-04T15:29:33Z

I will be away for work the upcoming week. I will then look into it.

Thank you. That will be fine

Adds support for reading, enabling, disabling, as well as generally updating Traffic Routes. I based this off the PR Kane610#409

Kane610 requested changes Aug 9, 2023

View reviewed changes

herriejr closed this Aug 22, 2023

herriejr force-pushed the master branch from e4cc0a0 to 787ad82 Compare August 22, 2023 19:37

herriejr reopened this Aug 26, 2023

Kane610 requested changes Aug 26, 2023

View reviewed changes

herriejr closed this Aug 28, 2023

herriejr force-pushed the master branch from 9178ed2 to e22a010 Compare August 28, 2023 19:21

herriejr reopened this Aug 28, 2023

Kane610 reviewed Aug 29, 2023

View reviewed changes

aiounifi/interfaces/traffic_rules.py Outdated Show resolved Hide resolved

aiounifi/interfaces/api_handlers.py Outdated Show resolved Hide resolved

aiounifi/models/api.py Outdated Show resolved Hide resolved

Kane610 requested changes Aug 31, 2023

View reviewed changes

aiounifi/models/api.py Outdated Show resolved Hide resolved

tests/conftest.py Show resolved Hide resolved

Kane610 requested changes Sep 1, 2023

View reviewed changes

Kane610 reviewed Sep 5, 2023

View reviewed changes

herriejr closed this Sep 27, 2023

herriejr force-pushed the master branch from 4ecfa11 to a5191e3 Compare September 27, 2023 19:05

Rebased the needed changes for the traffic rules on the rewrite of th…

ea8dcc3

…e aiounifi library.

Erwin Hermans added 2 commits September 29, 2023 13:03

Revert accidental deletion of 2 test-files

8e10574

Revert accidental deletion of 2 test-files & removed modifications

e09e2db

Alterations for solving the comments/suggestions on the pull request

5d953a4

Kane610 requested changes Sep 30, 2023

View reviewed changes

Kane610 reviewed Sep 30, 2023

View reviewed changes

aiounifi/models/api.py Outdated Show resolved Hide resolved

herriejr and others added 4 commits October 3, 2023 21:10

Update aiounifi/interfaces/api_handlers.py

3542564

Remove unnecessary import Co-authored-by: Robert Svensson <Kane610@users.noreply.github.com>

Update tests/test_ports.py

0b336b3

Remove unnecessary import Co-authored-by: Robert Svensson <Kane610@users.noreply.github.com>

Update tests/test_outlets.py

2d3ee1e

Remove unnecessary import Co-authored-by: Robert Svensson <Kane610@users.noreply.github.com>

Restructuring of the code, so that the most changes fit in the ApiReq…

64f4322

…uestV2-base class.

Kane610 approved these changes Oct 5, 2023

View reviewed changes

Kane610 reviewed Oct 5, 2023

View reviewed changes

tests/test_traffic_rules.py Show resolved Hide resolved

Adding test for the different methods of enabling and disabling the t…

37c8157

…raffic rules.

Kane610 approved these changes Oct 10, 2023

View reviewed changes

Kane610 merged commit d8dd288 into Kane610:master Oct 10, 2023
1 check passed

paravoid mentioned this pull request Nov 1, 2023

Traffic rules (v64) broken with old controller versions #517

Closed

ViViDboarder added a commit to ViViDboarder/aiounifi that referenced this pull request Nov 19, 2023

Add traffic routes

3507d21

Adds support for reading, enabling, disabling, as well as generally updating Traffic Routes. I based this off the PR Kane610#409

ViViDboarder mentioned this pull request Nov 19, 2023

Add traffic routes #529

Closed

Add traffic rules to library #409

Add traffic rules to library #409

Conversation

herriejr commented Jul 17, 2023

Kane610 commented Jul 25, 2023

Kane610 left a comment

Choose a reason for hiding this comment

herriejr commented Aug 22, 2023

Kane610 commented Aug 22, 2023

herriejr commented Aug 22, 2023 • edited

Kane610 commented Aug 23, 2023

herriejr commented Aug 26, 2023

Kane610 left a comment

Choose a reason for hiding this comment

Kane610 commented Aug 26, 2023 • edited

Kane610 commented Aug 27, 2023

herriejr commented Aug 28, 2023

Kane610 left a comment

Choose a reason for hiding this comment

herriejr commented Aug 30, 2023

herriejr commented Aug 31, 2023

Kane610 commented Aug 31, 2023

Kane610 left a comment

Choose a reason for hiding this comment

Kane610 left a comment

Choose a reason for hiding this comment

herriejr commented Sep 7, 2023

Kane610 commented Sep 9, 2023

Kane610 commented Sep 27, 2023

herriejr commented Sep 29, 2023

Kane610 left a comment

Choose a reason for hiding this comment

Kane610 commented Oct 3, 2023

Kane610 left a comment

Choose a reason for hiding this comment

Kane610 commented Oct 5, 2023

Kane610 left a comment

Choose a reason for hiding this comment

herriejr commented Oct 10, 2023

Kane610 commented Oct 10, 2023

ViViDboarder commented Oct 12, 2023

Kane610 commented Oct 12, 2023

Kane610 commented Oct 12, 2023

herriejr commented Oct 19, 2023

Kane610 commented Oct 24, 2023

Kane610 commented Nov 4, 2023 • edited

herriejr commented Nov 4, 2023

Kane610 commented Nov 4, 2023

herriejr commented Aug 22, 2023 •

edited

Kane610 commented Aug 26, 2023 •

edited

Kane610 commented Nov 4, 2023 •

edited