Skip to content

Fix resolving devices by defining the correct search key #136

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

Merged
merged 3 commits into from
Jul 12, 2025

Conversation

Talkabout
Copy link
Contributor

@Talkabout Talkabout commented Jul 11, 2025

When defining a device, for example, in the address module, it fails due to the wrong search key when calling find_by_key in _resolve_entity.

Due to the fields needed for a device entity in phpIPAM API:

$ curl -sk -XGET -H "phpipam-token: ${PHPIPAM_TOKEN}" ${PHPIPAM_API_URL}/tools/devices/ | jq '.data[0] | {id: .id, hostname: .hostname, ip: .ip}'
{
  "id": "8",
  "hostname": "Test device",
  "ip": "192.2.0.222"
}

By introducing an extra case for controller tools/devices we can override the default search key with hostname.

Talkabout and others added 2 commits July 12, 2025 01:41
To guarantee that the introduced change work as expected, a test case
`address_device_mapping` was added too.
Second a short fragment for the changelog was added.
@cmeissner cmeissner changed the title fixed resolving devices by defining the correct search key Fix resolving devices by defining the correct search key Jul 12, 2025
@cmeissner
Copy link
Member

@Talkabout, thank you for filing this pull request. I updated the description a bit for others to better follow what it will achieve.

I also added a test case for that special combination of address and device to be sure the code is working. Also, a short changelog fragment was added to have a corresponding entry in the changelog of the upcoming version.

After granting you the permissions to trigger actions to run on push, I also realized that something seems to be broken. I will try to investigate on this issue, if I found time. You are also invited to invest some time to get to the root cause of that failure in CI.

Due to timing problems with running tests in parallel with only 4
runners we increases the amount of runners to 8 to mitigate conflicting
actions.
This is only a workaround and the tests need to be refactored to prevent
conflicting actions by using random data.
@cmeissner cmeissner merged commit eb4ecd0 into codeaffen:develop Jul 12, 2025
5 checks passed
@Talkabout
Copy link
Contributor Author

@cmeissner Thanks for taking care!

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