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

reports: Update to use report_prefix #73

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

b4ldr
Copy link
Member

@b4ldr b4ldr commented Oct 2, 2023

This commit removes the report_filename config parameter and adds a report_prefix parameter. AFAICT the current code suggests that it will write all host data to the same file. However the code writes the old data to the prom file giving all values -1 which is incorrect

I looked at why this was and i can't find much information on the original issue[1] and PR[2]. further to this the current implementation is very racy if one sets REPORT_FILENAME

This also updates the clean stale function to only act on files with the report_prefix and avoid deleting files which may have been placed there by none puppetserver process

more then happy to split this up into separate patches but wanted to get a bit more feedback on why things got set to -1 and if im missing something

[1]#5 [2]#7

@b4ldr b4ldr force-pushed the report_prefix branch 4 times, most recently from 490115a to fd1991d Compare October 2, 2023 18:34
This commit removes the report_filename config parameter and adds a
report_prefix parameter.  AFAICT the current code suggests that it will
write all host data to the same file.  However the code writes the old
data to the prom file giving all values -1 which is incorrect

I looked at why this was and i can't find much information on the
original issue[1] and PR[2].  further to this the current implementation
is very racy if one sets REPORT_FILENAME

This also updates the clean stale function toi only act on files with
the report_prefix and avoid deleting files which may have been placed
there by none puppetserver process

[1]voxpupuli#5
[2]voxpupuli#7
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.

None yet

1 participant