handle error when log file is empty

[AliyevH]

Proposed Commit Message

feat: handle error when log file is empty

Fixes GH-4686

Additional Context

Test Steps

Checklist

• My code follows the process laid out in the documentation
• I have updated or added any unit tests accordingly
• I have updated or added any documentation accordingly

Merge type

• Squash merge using "Proposed Commit Message"
• Rebase and merge unique commits. Requires commit messages per-commit each referencing the pull request number (#<PR_NUM>)

[a-dubs]

Thank you for contributing! Since this is your first PR for cloud-init, please fill out the CLA form and then add your username to tools/.github-cla-signers and include that modification in this PR. See the Submit your first pull request page on cloud-init's docs for more information.

And then also be sure to add an appropriate commit message to your PR's Proposed Commit Message section. A commit message such as feat: handle error when log file is empty would suffice.

[AliyevH]

@a-dubs Already signed CLA. Added username and proposed commit message.
Thanks!

[TheRealFalcon]

@AliyevH , thanks for the contribution. While this does solve the problem, adding a sys.exit() into what is essentially a helper function several calls deep isn't ideal. It doesn't allow for any recovery for any other caller that might call the function.

Instead, can we raise a custom exception here (something like EmptyFileException), and then catch that exception at the bottom of cloudinit/analyze/__init__.py and print the proper text there?

[AliyevH]

@TheRealFalcon Thanks for review!

1. sys exit is called in load_events_infile function.
2. It has been called several times only in show.py
3. If file is empty i think it does not make sense to continue.
4. cloudinit/analyze/__init__.py here we can not catch exception, because this file is not called directly.
   if __name__ == "__main__":
   (sorry, maybe i am wrong. Trying to understand architecture. )

[TheRealFalcon]

cloudinit/analyze/init.py here we can not catch exception, because this file is not called directly.

Sorry about that. You're right. It's actually called from main.py.

I still don't love having a sys.exit(1) here, but doing it a different way would require some more substantial refactors which isn't fair to ask for in this PR. It is ok as-as.

I left one inline comment and can we also get a test? Since we don't already have a unit test file for cloud-init analyze show, you can create one at tests/unittests/analyze/test_show.py. It should look something like

from collections import namedtuple

import pytest

from cloudinit.analyze import analyze_show


@pytest.fixture
def mock_io(tmp_path):
    """Mock args for configure_io function"""
    infile = tmp_path / "infile"
    outfile = tmp_path / "outfile"
    return namedtuple("MockIO", ["infile", "outfile"])(infile, outfile)


class TestAnalyzeShow:
    """Test analyze_show (and/or helpers) in cloudinit/analyze/__init__.py"""

    def test_empty_logfile(self, mock_io, capsys):
        """Test analyze_show with an empty logfile"""
        mock_io.infile.write_text("")
        with pytest.raises(SystemExit):
            analyze_show("dontcare", mock_io)
        assert capsys.readouterr().err == f"Empty file {mock_io.infile}\n"

[TheRealFalcon]

LGTM! Thanks for the contribution.

[AliyevH]

Thank you for guiding me !