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

fix: Creating inventory with None host (re-running traceback) #177

Merged
merged 1 commit into from
May 12, 2022

Conversation

dav-pascual
Copy link
Member

@dav-pascual dav-pascual commented May 12, 2022

The code in create_invetory method was looping hostnames from the DB,
which some of them may have already been deleted, which causes a None return
when trying to get their metadata (get_host_from_metadata).

This fixes traceback caused by rerunning mrack up with a different host name
without previously deleting mrackdb.json

@Tiboris Tiboris self-requested a review May 12, 2022 08:37
@dav-pascual
Copy link
Member Author

dav-pascual commented May 12, 2022

The solution I came up with (the simplest) is checking None return from get_host_from_metadata function. This should prevent further traceback, when not deleting mrackdb.json between reruns.

An alternative to this solution would be to check if the status of the host is set to active, or using success_host list.

The code in create_invetory method was looping hostnames from the DB,
which some of them may have already been deleted, which cause a None return
when trying to get their metadata.

This fixes traceback caused by rerunning mrack up with a different host name
without previously deleting mrackdb.json

Signed-off-by: David Pascual <[email protected]>
@dav-pascual dav-pascual force-pushed the fix-image-none-value branch from 8282039 to fa1d3ab Compare May 12, 2022 08:45
@Tiboris Tiboris self-assigned this May 12, 2022
@pvoborni
Copy link
Contributor

Hi, I didn't test it. But conceptually the approach looks OK to me - we can go with it.

Just some thoughts: it is more a band-aid then a fix of root cause. E.g., we can easily face it in the future again somewhere else when this fix is "forgotten". I like the idea of success_host list, mainly from the perspective that it can serve as a basis for new features. But that is not needed now and might be better to do with the new features.

Copy link
Member

@Tiboris Tiboris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patch @dav-pascual lets merge it.

@dav-pascual
Copy link
Member Author

Thanks. Let's merge this quick patch for now, with a view on implementing a more robust solution regarding the cleaning of previous runs.

@dav-pascual dav-pascual merged commit 7489240 into neoave:master May 12, 2022
@dav-pascual dav-pascual deleted the fix-image-none-value branch May 12, 2022 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants