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

checking on an undefined snmpid, do not include those into the mrtg conf... #200

Closed
wants to merge 1 commit into from

Conversation

fooelisa
Copy link
Contributor

belts and braces barryo :)

@barryo
Copy link
Member

barryo commented Feb 18, 2015

Hey @fooelisa! I'm not sure the following test is the best:

if( $port->ifnameToSNMPIdentifier() )

That function just sanitises the ifname for MRTG. If I understand correctly, you have cases where there is no ifname? If so, the test would be better written as:

if( $port->getIfName() )

This would evaluate as false for null and empty string.

  • Barry

@fooelisa
Copy link
Contributor Author

You are absolutely right, they don't have a name. When I entered the interfaces I put in something called name, but on second glance it seems like this became the "name" not the "ifName" in the DB. This works for me! Thanks

@barryo barryo closed this in 3674006 Feb 18, 2015
@barryo
Copy link
Member

barryo commented Feb 18, 2015

Done.

When I entered the interfaces I put in something called name

We should really remove the code that allows you to add ports manually. This only leads to problems. Ports should be discovered and added:

  • when adding the switch by SNMP discovery
  • as part of a cron script that re-discovers switches routinely.

@fooelisa
Copy link
Contributor Author

Agreed, although, It came in handy in my case for a reseller port who I entered and assigned a fake interface for. Not having an interface broke the brid config generation, which could probably use some love in this scenario as well.

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