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 dependency info for device list in api calls #8058

Merged
merged 5 commits into from Jan 14, 2018

Conversation

Projects
None yet
4 participants
@aldemira
Contributor

aldemira commented Jan 9, 2018

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

aldemira added some commits Jan 9, 2018

@aldemira aldemira changed the title from Add dependency info for api to Add dependency info for device list in api calls Jan 9, 2018

@@ -315,6 +315,15 @@ function list_devices()
}
$devices = array();
foreach (dbFetchRows("SELECT * FROM `devices` $join WHERE $sql ORDER by $order", $param) as $device) {
$dev_deps = dbFetchRows("SELECT parent_device_id from device_relationships WHERE child_device_id = ?", array($device));

This comment has been minimized.

@murrant

murrant Jan 9, 2018

Member

dbFetchColumn puts a single column in a nice array ;)

@@ -315,6 +315,11 @@ function list_devices()
}
$devices = array();
foreach (dbFetchRows("SELECT * FROM `devices` $join WHERE $sql ORDER by $order", $param) as $device) {
$dev_deps = dbFetchColumn("SELECT parent_device_id from device_relationships WHERE child_device_id = ?", array($device));

This comment has been minimized.

@laf

laf Jan 9, 2018

Member

I already expect that people will ask for hostnames as well as ID's. Here's a totally unformatted but working example:

select d.device_id, d.hostname, GROUP_CONCAT(dd.device_id), GROUP_CONCAT(dd.hostname) as parent_hostname from devices as d left join device_relationships as dr on dr.child_device_id=d.device_id left join devices as dd on dr.parent_device_id=dd.device_id GROUP BY d.hostname

@murrant

This comment has been minimized.

Member

murrant commented Jan 9, 2018

Any thoughts on this being under it's own route? Or some way to mark with parents or children?

@laf

This comment has been minimized.

Member

laf commented Jan 10, 2018

Any thoughts on this being under it's own route? Or some way to mark with parents or children?

How come?

@aldemira

This comment has been minimized.

Contributor

aldemira commented Jan 11, 2018

I know using sizeof() is a bit silly, but empty() and if(!) doesn't work. Let me know if you have a better solution.

@laf

This comment has been minimized.

Member

laf commented Jan 11, 2018

@aldemira What about my comment about the query?

I already expect that people will ask for hostnames as well as ID's. Here's a totally unformatted but working example:

select d.device_id, d.hostname, GROUP_CONCAT(dd.device_id), GROUP_CONCAT(dd.hostname) as parent_hostname from devices as d left join device_relationships as dr on dr.child_device_id=d.device_id left join devices as dd on dr.parent_device_id=dd.device_id GROUP BY d.hostname

@aldemira

This comment has been minimized.

Contributor

aldemira commented Jan 11, 2018

@laf Yeah, I got it and simplified it so that it is a JSON array like
[{ device_id: X, hostname: aaa}, {device_id: Y, hostname: bbb}]
Isn't this what you wanted?

@laf

This comment has been minimized.

Member

laf commented Jan 11, 2018

Isn't this what you wanted?

Nope :) The new query was to replace the single device lookup so you only do one query to get devices and dependencies in one go :D

@aldemira

This comment has been minimized.

Contributor

aldemira commented Jan 11, 2018

@laf I'm sorry, now I get it :)

@scrutinizer-notifier

This comment has been minimized.

scrutinizer-notifier commented Jan 12, 2018

The inspection completed: 3 new issues, 3 updated code elements

@laf

This comment has been minimized.

Member

laf commented Jan 14, 2018

Ok wasn't sure on the formatting of this in the end after seeing it returned as part of the json.

So: https://p.libren.ms/view/raw/a591f48e

Output below. I have removed the p.* from the select as I don't think we should be returning all the data for mac, ipv4 or ipv6 if someone is searching on it and just list the device info.

I've also added this to the get_device call so it works on single devices:

{
    "status": "ok",
    "devices": [
        {
            "device_id": "366",
            "hostname": "laf2",
            "sysName": "laf2",
            "ip": null,
            "community": "librenms",
            "authlevel": null,
            "authname": null,
            "authpass": null,
            "authalgo": null,
            "cryptopass": null,
            "cryptoalgo": null,
            "snmpver": "v2c",
            "port": "161",
            "transport": "udp",
            "timeout": null,
            "retries": null,
            "snmp_disable": "0",
            "bgpLocalAs": null,
            "sysObjectID": null,
            "sysDescr": null,
            "sysContact": null,
            "version": null,
            "hardware": "",
            "features": null,
            "location": null,
            "os": "generic",
            "status": "0",
            "status_reason": "icmp",
            "ignore": "0",
            "disabled": "0",
            "uptime": null,
            "agent_uptime": "0",
            "last_polled": null,
            "last_poll_attempted": null,
            "last_polled_timetaken": null,
            "last_discovered_timetaken": null,
            "last_discovered": null,
            "last_ping": null,
            "last_ping_timetaken": null,
            "purpose": null,
            "type": "server",
            "serial": null,
            "icon": null,
            "poller_group": "0",
            "override_sysLocation": "0",
            "notes": null,
            "port_association_mode": "1",
            "attribs": [],
            "dependancy": [
                {
                    "hostname": "laf",
                    "parent_device_id": "168"
                },
                {
                    "hostname": "localhost",
                    "parent_device_id": "398"
                }
            ]
        }
    ],
    "count": 1
}
@laf

This comment has been minimized.

Member

laf commented Jan 14, 2018

@murrant how do you feel about the above?

@laf laf added the API label Jan 14, 2018

@murrant

This comment has been minimized.

Member

murrant commented Jan 14, 2018

Looks fine to me.

@laf laf merged commit 788c0bd into librenms:master Jan 14, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

@aldemira aldemira deleted the aldemira:hostdeps4api branch Feb 22, 2018

@lock

This comment has been minimized.

lock bot commented May 16, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed.

@lock lock bot locked as resolved and limited conversation to collaborators May 16, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.