@tchaikov Would it be wise to implement our own struct fmt::formatter<clog_type> instead of providing a formatter inherited from ostream_formatter here ?

@pdvian hi Prashant, I think it depends on if we are still using operator<< to print this type. Please note, if we have both of them if the tree is compiled with fmtlib< 9.0. Personally, I'd avoid replicate the formatting logic in two places unless one of the implementations is very readable/ simple.

@pdvian hi Prashant, I think it depends on if we are still using operator<< to print this type. Please note, if we have both of them if the tree is compiled with fmtlib< 9.0. Personally, I'd avoid replicate the formatting logic in two places unless one of the implementations is very readable/ simple.

Thanks. It makes sense.

@tchaikov Hi Kefu, Can you review this PR please ?

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

This pull request has been automatically closed because there has been no activity for 90 days. Please feel free to reopen this pull request (or open a new one) if the proposed change is still appropriate. Thank you for your contribution!

Re-opening this PR as we are encountering this issue in teuthology tests. e.g https://pulpito.ceph.com/yuriw-2024-01-10_19:18:18-rados-wip-pdhange-testing-distro-default-smithi/7512554/

@rzarzynski @NitzanMordhai Kindly review this PR.

Hi @pdvian, Another PR: #55455 was recently raised and reviewed to fix this. PTAL.

Hi @pdvian, Another PR: #55455 was recently raised and reviewed to fix this. PTAL.

Thanks Sridhar. Your PR looks good to me. I am fine to close this PR in favor of your PR.

@rzarzynski @NitzanMordhai Sridhar has another PR #55455 ready to merge for the same issue. Should we go ahead with str helper function instead of defining formatter for clog_type ?

Thanks Sridhar. Your PR looks good to me. I am fine to close this PR in favor of your PR.

@rzarzynski @NitzanMordhai Sridhar has another PR #55455 ready to merge for the same issue. Should we go ahead with str helper function instead of defining formatter for clog_type ?

@pdvian @rzarzynski Can both the fixes co-exist? This is only if there's a possibility that fmt version can be lower than 90000. Otherwise, I am fine with whatever you think is the best fix.

@sseshasa , @rzarzynski , @tchaikov

@pdvian @rzarzynski Can both the fixes co-exist? This is only if there's a possibility that fmt version can be lower than 90000. Otherwise, I am fine with whatever you think is the best fix.

Aren't we are passed this point in Squid?

Thanks Sridhar. Your PR looks good to me. I am fine to close this PR in favor of your PR.
@rzarzynski @NitzanMordhai Sridhar has another PR #55455 ready to merge for the same issue. Should we go ahead with str helper function instead of defining formatter for clog_type ?

@pdvian @rzarzynski Can both the fixes co-exist? This is only if there's a possibility that fmt version can be lower than 90000. Otherwise, I am fine with whatever you think is the best fix.

Yes, both fixes can co-exist. @rzarzynski What do you think ?

@sseshasa , @rzarzynski , @tchaikov

@pdvian @rzarzynski Can both the fixes co-exist? This is only if there's a possibility that fmt version can be lower than 90000. Otherwise, I am fine with whatever you think is the best fix.

Aren't we are passed this point in Squid?

The fmt submodule in ceph repo is having FMT_VERSION 90100 so we need to define fmt::formatter for clog_type explicitly.

Teuthology test result

https://pulpito.ceph.com/yuriw-2024-03-08_16:20:46-rados-wip-yuri4-testing-2024-03-05-0854-distro-default-smithi/.

There are no related failures. For the full report please see: https://tracker.ceph.com/projects/rados/wiki/MAIN#httpstrellocomcQFtoIRXE1971-wip-yuri4-testing-2024-03-05-0854

This is RADOS approved.