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

Cannot disable bad-import-yang-version #40

Open
Sparrow1029 opened this issue May 20, 2021 · 9 comments
Open

Cannot disable bad-import-yang-version #40

Sparrow1029 opened this issue May 20, 2021 · 9 comments

Comments

@Sparrow1029
Copy link

Thank you for making this extension!

I have YANG models that I am using with Cisco NSO (Network Services Orchestrator), and cannot disable bad-import-yang-version error.

I have set up my ~/.yang/yang.settings file like this:

{
  "yangPath": "/Path/to/ietf:/tailf/modules",
  "diagnostic": {
    "bad-import-yang-version": "ignore",
    "ambiguous-import": "ignore"
  }
}

I have also tried placing this same configuration into yang.settings at my project root.

Setting "ambiguous-import" to "ignore" works just fine, as those warnings are now disabled.

I currently have some "duplicate-name" errors in my YANG, and as an experiment, successfully changed/disabled those errors using yang.settings as well.

The bad-import-yang-version errors do not disappear until I explicitly put

module my-module {
    yang-version 1.1;

at the top of the module (which isn't usually necessary for Cisco NSO to infer the version).

Adding the explicit yang-version leaf at the top of the modules isn't a huge pain, but wanted to report this behavior. I am wondering if it is a bug, or if I should be using some other configuration.

@vk496
Copy link

vk496 commented May 5, 2022

Same for me

@dhuebner
Copy link
Member

dhuebner commented Jul 7, 2022

@Sparrow1029
Empty yang-version assumes version 1.0

This kind of validation bad-import-yang-version and bad-include-yang-version is currently explicitly set to error and is not configurable.
I could change that, but I'm not sure yet if it can break the consistency of the LS.

https://datatracker.ietf.org/doc/html/rfc7950#section-12

@esmasth
Copy link
Contributor

esmasth commented Mar 2, 2024

Hello @dhuebner, I have a related issue on TypeFox/yang-lsp#228 where it is requested to refine the diagnostic to match the RFC, as it is allowed to import yang-version 1.1 models from yang-version 1.0 models as long as the import is not with a revision-date.

Also, this ticket was helpful, since I had struggled in isolation when trying to downgrade bad-import-yang-version to a warning severity, due to the false positive against the RFC compliant import behavior.

@dhuebner
Copy link
Member

dhuebner commented Mar 4, 2024

@esmasth
We need to make severity for bad-import-yang-version and bad-include-yang-version validation configurable. Unfortunately I don't have much time currently to implement this now.
Maybe you could talk to @RimShao to change the priorities, so we can work on this issue.

@esmasth
Copy link
Contributor

esmasth commented Mar 6, 2024

Thanks @dhuebner, I have discussed with Rim and this is rather an independent engagement from my side hence it should not not interfere. I may provide a draft PR for consideration, if welcome, for TypeFox/yang-lsp#228 when I get my development environment up and running.

esmasth added a commit to esmasth/yang-lsp that referenced this issue Mar 12, 2024
Based on discussion in TypeFox/yang-vscode#40 bad-include-yang-version
and bad-import-yang-version are not configurable.

Also updated Settings.md documentation to clarify.

Signed-off-by: Siddharth Sharma <[email protected]>
@dhuebner
Copy link
Member

Making validation configurable bei the user is pretty easy, see:
TypeFox/yang-lsp#237 (comment)

@esmasth
Copy link
Contributor

esmasth commented Mar 24, 2024

Thanks @dhuebner . The other cases of using error(...) in yang-lsp seemed to be consistent with MUST/SHALL/... statements in the YANG RFCs on a quick check. Unless there's a motivation brought forth on why bad-import-yang-version and bad-include-yang-version need to be worked around, especially after the fix for TypeFox/yang-lsp#228, it would seem to me that configurability could be limited to SHOULD/RECOMMENDED/..., best practices, and inconsistent spec/implementation domain.

@dhuebner
Copy link
Member

dhuebner commented Apr 2, 2024

@esmasth
If you already have a list of SHOULD/RECOMMENDED/ validation to make configurable, maybe you can provide a PR for that?

@esmasth
Copy link
Contributor

esmasth commented Apr 3, 2024

@dhuebner it would need a dedicated exercise to comb through the layers of RFC text to create that definitive list. I don't have that readily available for contribution, sorry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants