Skip to content

Conversation

@mkalcok
Copy link
Contributor

@mkalcok mkalcok commented Apr 25, 2025

Few places in the code made assumption that for subordinate charms, the 'subordinate-to' list would contain only principal charms. However that is not the case. If a suboridnate charm is related to other subordinate charms, they show up in each other's 'subordinate-to' lists.

This was especially problematic in the 'get_machines_for_application' function which would end up in an infinite loop.

@mkalcok mkalcok requested a review from fnordahl April 25, 2025 14:52
@mkalcok mkalcok force-pushed the get-machine-recursion branch from 0d491ab to 6a7667d Compare April 30, 2025 10:54
@mkalcok
Copy link
Contributor Author

mkalcok commented Apr 30, 2025

I found one more place (async_block_until_unit_wl_status in model.py) that made wrong assumptions about subordinate-to list, so I updated the PR.

mkalcok added 3 commits April 30, 2025 13:52
Direct calls to setup.py have been deprecated [0] and
they recently started to cause functional test to fail with
error:

NotImplementedError: Support for egg-based install has been removed.

This change removes explicit setup.py calls from tox commands, as it
shouldn't be necessary to explicitly install local project into the
tox virtual environment anyway.

[0] https://blog.ganssle.io/articles/2021/10/setup-py-deprecated.html

Signed-off-by: Martin Kalcok <[email protected]>
Per deprecation notice from setuptools, multi-word keys that
use dashes to separate words are deprecated and will stop working
by 2026-Mar-03. The dashes should be replaced with underscores [0].

[0] https://setuptools.pypa.io/en/latest/userguide/declarative_config.html

Signed-off-by: Martin Kalcok <[email protected]>
Few places in the code made assumption that for subordinate charms,
the 'subordinate-to' list would contain only principal charms. However
that is not the case. If a suboridnate charm is related to other
subordinate charms, they show up in each other's 'subordinate-to' lists.

This was especially problematic in the 'get_machines_for_application'
function which would end up in an infinite loop.

Signed-off-by: Martin Kalcok <[email protected]>
@mkalcok mkalcok force-pushed the get-machine-recursion branch from 6a7667d to a361300 Compare April 30, 2025 11:52
@mkalcok
Copy link
Contributor Author

mkalcok commented Apr 30, 2025

Functional test dependency installation randomly broke since last week, so tox file got updated as well.

Copy link
Collaborator

@fnordahl fnordahl left a comment

Choose a reason for hiding this comment

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

LGTM, and thank you for the code gardening, leaving the tree in a better shape than when you found it!

@fnordahl fnordahl merged commit 83cd750 into openstack-charmers:master Apr 30, 2025
8 checks passed
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