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

Add log with OS app string representation to Analysis._populate #368

Closed
wants to merge 5 commits into from

Conversation

rgildein
Copy link
Contributor

With this, we can collect info about application during Analysis._populate, like this the log will contain the app string representation before planning.

log message example:

found OpenStack application:
app-name:
  model_name: <model-name>
  ...

With this, we can collect info about application during Analysis._populate, like this the log will contain the app string representation before planning.

log message example:
```bash
found OpenStack application:
app-name:
  model_name: <model-name>
  ...
```
@rgildein rgildein added the enhancement New feature or request label Apr 10, 2024
@rgildein rgildein self-assigned this Apr 10, 2024
@rgildein rgildein requested a review from a team as a code owner April 10, 2024 16:06
for name, app in juju_applications.items():
if os_app := AppFactory.create(app):
apps.add(os_app)
logger.info("found OpenStack application:\n%s", os_app)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
logger.info("found OpenStack application:\n%s", os_app)
logger.info("Found OpenStack application:\n%s", os_app)

Copy link
Contributor

@agileshaw agileshaw left a comment

Choose a reason for hiding this comment

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

Two minor things


# remove non-supported charms that return None on AppFactory.create
apps.discard(None)
# mypy complains that apps can have None, but we already removed.
Copy link
Contributor

Choose a reason for hiding this comment

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

we need to remove this comment too

}
other_o7k_apps = apps - apps_to_upgrade_in_order
sorted_apps_to_upgrade_in_order = sorted(
apps_to_upgrade_in_order,
key=lambda app: UPGRADE_ORDER.index(app.charm), # type: ignore
key=lambda app: UPGRADE_ORDER.index(app.charm),
)
# order by charm name to have a predictable upgrade sequence of others o7k charms.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# order by charm name to have a predictable upgrade sequence of others o7k charms.
# order by charm name to have a predictable upgrade sequence of other o7k charms.

@aieri
Copy link
Contributor

aieri commented Apr 10, 2024

superseded by #371

@aieri aieri closed this Apr 10, 2024
@rgildein rgildein deleted the rgildein-patch-1 branch April 10, 2024 20:56
rgildein pushed a commit that referenced this pull request Apr 10, 2024
With this, we can collect info about application during
Analysis._populate, like this the log will contain the app string
representation before planning.

log message example:
```
found OpenStack application:
app-name:
  model_name: <model-name>
  ...
```

This PR supersedes #368
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants