-
Notifications
You must be signed in to change notification settings - Fork 311
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
Simplify Packet Broker settings #6639
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested this locally and I have a few issues:
- If you select sandbox, it creates a policy with NetID
000013
but not tenant IDttn
. It should be with tenant IDttn
explicitly because that is the community network. - When I go from the first radio button (forward traffic to all networks) and save, there's indeed a default routing policy created. However, the home network routing policy with
00013
is still there. I think all other policies should be deleted - When I go back to the second radio button (forward to sandbox) and click save, I get the error [0]
[0]
{
"code": 3,
"message": "type mismatch, parameter: net_id, error: strconv.ParseUint: parsing \"NaN\": invalid syntax"
}
I checked the staging networks list and it seems like The Things Network network does not have a tenant id. Also when updating its policy the request is sent to
|
2661483
to
1761a1d
Compare
TTN is not on a tenant, is it? It's a single-tenant network, or what am I missing? |
Yeah it actually is a tenant, but not on The Things Stack Cloud. The Things Stack Sandbox is a single tenant deployment that uses NetID @ryaplots indeed the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default gateway visibility configuration should not be deleted but moved to a separate tab next to "Routing configuration".
pkg/webui/console/components/default-routing-policy-form/index.js
Outdated
Show resolved
Hide resolved
pkg/webui/console/components/default-routing-policy-form/routing-policy-form.styl
Outdated
Show resolved
Hide resolved
pkg/webui/console/views/admin-packet-broker/admin-packet-broker.js
Outdated
Show resolved
Hide resolved
pkg/webui/console/views/admin-packet-broker/default-gateway-visibility.js
Outdated
Show resolved
Hide resolved
Where can I get the texts from the tooltips of the routing options? @kschiffer |
Let's skip them for now. |
We need to update the routing there in general a bit and make I've taken a shot at this and added a fix commit. |
c0b533a
to
5a86cfc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do a final test of the new code before merging.
|
I have taken this into account and added back the functionality. @johanstokking Please review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this now works as expected.
Summary
Closes #3838
Changes
Testing
Checklist
README.md
for the chosen target branch.CHANGELOG.md
.CONTRIBUTING.md
, there are no fixup commits left.