Skip to content

THREESCALE-14619: Remove the invalid default value for CONFIG_LOG_PATH#462

Open
mayorova wants to merge 1 commit into
masterfrom
fix-dev-stdout-backend-cron
Open

THREESCALE-14619: Remove the invalid default value for CONFIG_LOG_PATH#462
mayorova wants to merge 1 commit into
masterfrom
fix-dev-stdout-backend-cron

Conversation

@mayorova
Copy link
Copy Markdown
Contributor

@mayorova mayorova commented Apr 22, 2026

Fixes https://redhat.atlassian.net/browse/THREESCALE-14619

dev/stdout is an invalid log path. Removing the setting results in using other defaults for the log, and in OpenShift environment it defaults to STDOUT anyway, so we don't need to set the variable.

jlledom
jlledom previously approved these changes Apr 22, 2026
Comment thread openshift/backend-cron Outdated
# When running this executable STDOUT is more convenient to check that
# everything is working correctly.
ENV['CONFIG_LOG_PATH'] ||= 'dev/stdout'
ENV['CONFIG_LOG_PATH'] ||= '/dev/stdout'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what will happen if we don't set this variable? Will it log to stdout by default? It is worth trying because this is how https://redhat.atlassian.net/browse/THREESCALE-12057 was fixed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Well, this is what the PR fixes - if the variable is NOT set, previously, the error from the description is printed.
With this updated default, the log seems to be printed to the stdout successfully. This is what the backend-cron pod log looks like when CONFIG_LOG_PATH=/dev/stdout is set on the pod explicitly:

fatal: not a git repository (or any of the parent directories): .git
fatal: not a git repository (or any of the parent directories): .git
fatal: not a git repository (or any of the parent directories): .git
I, [2026-04-22T07:49:17.829096 #21] INFO -- : Going to delete the stats keys for these services: ["5"]
I, [2026-04-22T07:49:17.832578 #21] INFO -- : Finished deleting the stats keys for these services: ["5"]
** Invoke reschedule_failed_jobs (first_time)
** Execute reschedule_failed_jobs
Rescheduled: 0. Failed and discarded: 0. Pending failed jobs: 0.
** Invoke stats:cleanup (first_time)
** Execute stats:cleanup
fatal: not a git repository (or any of the parent directories): .git
** Invoke reschedule_failed_jobs (first_time)
** Execute reschedule_failed_jobs
Rescheduled: 0. Failed and discarded: 0. Pending failed jobs: 0.
fatal: not a git repository (or any of the parent directories): .git
** Invoke reschedule_failed_jobs (first_time)
** Execute reschedule_failed_jobs
Rescheduled: 0. Failed and discarded: 0. Pending failed jobs: 0.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

previously it was not unset but it was set to dev/stdout which expectedly cannot be found.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Errno::ENOENT: No such file or directory @ rb_file_s_stat - dev/stdout (Errno::ENOENT)

This means it cannot find dev/stdout.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think the change is not wrong. According to this:

# often we don't have a log_file setting - generate it here from
# the log_path setting.
log_file = config.log_file
if !log_file || log_file.empty?
log_path = config.log_path
config.log_file = if log_path && !log_path.empty?
if File.stat(log_path).ftype == 'directory'
"#{log_path}/backend_logger.log"
else
log_path
end
else
ENV['CONFIG_LOG_FILE'] || STDOUT
end
end

This change would end up actually logging to /dev/stdout. I think removing the whole line would lead to the same result anyway, but I don't have the energy to test it and confirm it right now. IMO we should just merge this, fix the immediate problem and maybe review the whole logging system later on.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ok, I prefer not to have redundant settings in there. Also the variable doesn't seem to be used as intended as a PATH. But not super strong opinion. You can merge as you like it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@akostadinov is right, not setting the env var has the same effect - STDOUT is picked by default.

@mayorova mayorova force-pushed the fix-dev-stdout-backend-cron branch from 4da70e4 to 22eb705 Compare May 21, 2026 13:40
@mayorova mayorova changed the title THREESCALE-14619: Fix the default value for CONFIG_LOG_PATH THREESCALE-14619: Remove the invalid default value for CONFIG_LOG_PATH May 21, 2026
@mayorova mayorova requested review from akostadinov and jlledom May 21, 2026 15:19
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.

3 participants