Skip to content

Conversation

@safianalicb
Copy link

Logging + stats in cbbackupmgr has (mostly) been moved to the repository-level. This change makes promtimer compatible with these changes.

Copy link
Contributor

@dave-finlay dave-finlay left a comment

Choose a reason for hiding this comment

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

Hi Saf: the change is pretty straightforward and it works -- at least it did when I got the correct version of cbmstatparser built. :-)

One thing that wasn't clear to me was the distinction between "archive-level" stats and "repo-level" stats. Archive-level makes sense when these were the only places that stats could reside in an archive (under logs/stats) but now because stats may be found under any file path that matches*/logs/stats basically there are lots of "archive-level" stats.

Is "repo" a backup repository -- and is it the case that backup archives can now include backup information from multiple different backup repostories?

Copy link
Contributor

@dave-finlay dave-finlay left a comment

Choose a reason for hiding this comment

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

Patch is fine and works. Interested in the response to my question - perhaps the comments could be a bit clearer. Let me know your thoughts.

Comment on lines +702 to +705
for file in os.listdir(cpu_stats_dir):
file_path = path.join(cpu_stats_dir, file)
if path.isfile(file_path) and file[0] != '.':
stat_files.append(file_path)
Copy link
Contributor

Choose a reason for hiding this comment

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

You could use glob in some form here. I.e. something like:

files = glob.glob(f'{archive-path}/logs/stats/cpu/*')
files += glob.glob(f'{archive-path}/*/logs/stats/cpu/*')

It's not needed - of course it's fine to add files one-at-a-time, but maybe if you're back changing this code later it's something you could think about.

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