Skip to content

corosync config parser minor changes #1415

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

Merged

Conversation

liangxin1300
Copy link
Collaborator

@liangxin1300 liangxin1300 commented May 7, 2024

  • corosync_config_format: Add new line between sections
  • corosync_config_format: Enable to parse comments
    Add these two lines to the header
    # Generated by crmsh
    # For more details please see corosync.conf.5 man page
    

Copy link

codecov bot commented May 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 53.74%. Comparing base (42ad4f1) to head (fd2d806).
Report is 6 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1415      +/-   ##
==========================================
+ Coverage   53.63%   53.74%   +0.10%     
==========================================
  Files          80       80              
  Lines       24081    24058      -23     
==========================================
+ Hits        12917    12931      +14     
+ Misses      11164    11127      -37     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@nicholasyang2022 nicholasyang2022 left a comment

Choose a reason for hiding this comment

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

  1. Please add new unit test cases for comments.
  2. Please add some comments to the data in test case TestParseSerialize.

Comment on lines 44 to 45
fake_secion_name = f'{COMMENT_PREFIX}_{lineno}'
self.on_key_value(fake_secion_name, tokens[2])
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
fake_secion_name = f'{COMMENT_PREFIX}_{lineno}'
self.on_key_value(fake_secion_name, tokens[2])
fake_key = f'{COMMENT_PREFIX}_{lineno}'
self.on_key_value(fake_key, tokens[2])

self.on_value(value)
self._ofile.write('\n')
if key.startswith(COMMENT_PREFIX):
self._ofile.write(f'# {value}\n')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
self._ofile.write(f'# {value}\n')
self.__write_indent(len(self._path_stack))
self._ofile.write('# ')
self._ofile.write(value)
self._ofile.write('\n')
  1. Writing to buffered io directly tends to provide better performance.
  2. Indentation is needed

@@ -8,12 +8,13 @@


logger = logging.getLogger(__name__)
COMMENT_PREFIX = '__comment'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
COMMENT_PREFIX = '__comment'
COMMENT_PREFIX = '#comment'

Using # will make sure it does not conflict with anything.

@liangxin1300 liangxin1300 force-pushed the 20240507_corosync_parser_improve branch from fd2d806 to 258d56e Compare May 7, 2024 13:22
@liangxin1300
Copy link
Collaborator Author

  1. Please add new unit test cases for comments.
  2. Please add some comments to the data in test case TestParseSerialize.

Changed

@liangxin1300 liangxin1300 merged commit ec25d36 into ClusterLabs:master May 8, 2024
30 checks passed
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