这是indexloc提供的服务,不要输入任何密码
Skip to content

fix routing_table, it should be int as per the schema #34

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

Closed
wants to merge 1 commit into from

Conversation

gsimko
Copy link

@gsimko gsimko commented Apr 28, 2024

No description provided.

@OJFord
Copy link
Owner

OJFord commented May 20, 2024

Does this stem from some kind of issue? The schema description does say 'or "off"', which (though I've forgotten context and would need to look into it again) suggests to me this was deliberate, that it's either an int identifier or the string 'off', and therefore does need to be string?

@gsimko
Copy link
Author

gsimko commented May 21, 2024

I've already forgotten the details but the code was failing for sure.
I was referring to the schema in your code:

"routing_table": {
  Description: "Controls the routing table (or \"off\") to which routes are added. (`wg-quick`/apps only.)",
  Type:        schema.TypeInt,
  Optional:    true,
},

So maybe just updating the type to schema.TypeString would work fine?

@OJFord
Copy link
Owner

OJFord commented May 21, 2024

Yeah, so was I - that (or \"off\") - no worries, I'll have a look later.

@OJFord OJFord closed this in 30f299e May 21, 2024
OJFord added a commit that referenced this pull request May 21, 2024
and inability to use non-int named tables (e.g. `default`).

Closes #34.
@OJFord
Copy link
Owner

OJFord commented May 21, 2024

Thanks for bringing that to my attention @gsimko - though correct fix seems to change the schema, since the table could be for example "default". I've released v0.3.0 with that fix.

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.

2 participants