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

Add support for Nokia ISAM #9793

Merged
merged 9 commits into from Feb 13, 2019

Conversation

Projects
None yet
4 participants
@vitalisator
Copy link
Contributor

commented Feb 7, 2019

DO NOT DELETE THIS TEXT

Please note

Please read this information carefully. You can run ./scripts/pre-commit.php to check your code before submitting.

Testers

If you would like to test this pull request then please run: ./scripts/github-apply <pr_id>, i.e ./scripts/github-apply 5926
After you are done testing, you can remove the changes with ./scripts/github-remove. If there are schema changes, you can ask on discord how to revert.

@kkrumm1 kkrumm1 added the Device 🖥 label Feb 7, 2019

@murrant

This comment has been minimized.

Copy link
Member

commented Feb 7, 2019

Can you remove the commented lines of code?
A multi-get on for the device os polling is more efficient (but some devices don't like that).

vitalisator added some commits Feb 8, 2019

@vitalisator

This comment has been minimized.

Copy link
Contributor Author

commented Feb 8, 2019

@murrant, done

@murrant

This comment has been minimized.

Copy link
Member

commented Feb 11, 2019

@vitalisator did you intentionally remove the copyright/license comments?

@vitalisator

This comment has been minimized.

Copy link
Contributor Author

commented Feb 11, 2019

@vitalisator did you intentionally remove the copyright/license comments?
no, I was not sure which comments did you meant in your last post :-)
anyway, for me its is not important to have any copyright comments there.

Tests failing

@vitalisator

This comment has been minimized.

Copy link
Contributor Author

commented Feb 12, 2019

@murrant , thanks. I have overseen that. But Travis is still failed?

@PipoCanaja

This comment has been minimized.

Copy link
Contributor

commented Feb 12, 2019

Some test data must be cleaned as shown by travis. Tests were identifying devices as timos and should now change to nokia-isam.
You can run scripts/pre-commit.php -o timos and scripts/pre-commit.php -o nokia-isam and correct anything that must be corrected.

@murrant

This comment has been minimized.

Copy link
Member

commented Feb 12, 2019

I think the discovery is still wrong. Right now it says all Nokia devices that aren't 1.3.6.1.4.1.6527.1 are ISAM.

It seems like Nokia is pretty good about the sysDescr, I think it should be just:

    - sysDescr: NOKIA ISAM
    - sysObjectID:
        - .1.3.6.1.4.1.637.61

vitalisator added some commits Feb 12, 2019

first deactivate checks against IHUB (timos), It not necessary so lon…
…g we can not provide more thatn one snmp community per device
@vitalisator

This comment has been minimized.

Copy link
Contributor Author

commented Feb 12, 2019

not existing Test for bad_iftype in yaml file?
nokia-isam.yaml does not validate. Violations:
[] The property bad_iftype is not defined and the definition does not allow additional properties

@murrant

This comment has been minimized.

Copy link
Member

commented Feb 12, 2019

@vitalisator the schema is incomplete built to accommodate existing definitions (then expanded some). I'd appreciate it if you updated the schema.

@murrant

This comment has been minimized.

Copy link
Member

commented Feb 13, 2019

I updated the schema

@murrant
Copy link
Member

left a comment

Looks good :)

@murrant murrant merged commit d60a0da into librenms:master Feb 13, 2019

5 of 6 checks passed

codeclimate 7 issues to fix
Details
Inspection Summary
Details
Node: analysis
Details
Travis CI - Pull Request Build Passed
Details
WIP Ready for review
Details
license/cla Contributor License Agreement is signed.
Details

@vitalisator vitalisator deleted the vitalisator:nokia-isam branch Feb 16, 2019

@lock lock bot locked as resolved and limited conversation to collaborators Apr 17, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.