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

Clarify None handling for configuration parsing #126

Open
Kircheneer opened this issue Jun 29, 2022 · 3 comments
Open

Clarify None handling for configuration parsing #126

Kircheneer opened this issue Jun 29, 2022 · 3 comments

Comments

@Kircheneer
Copy link
Contributor

BaseSpaceConfigParser.build_config_relationship is using multiple ._build_$something methods that currently return an Optional[str]. It is not clear what happens, if one of those actually returns None. We should decide on whether we want to raise an exception in those specific methods or pass the None up the chain and handle it there. In any case, it needs to be handled to get rid of all the # type: ignore comments.

Originally posted by @Kircheneer in #123 (comment)

@Kircheneer
Copy link
Contributor Author

Also applicable to NokiaConfigParser._get_section_title

@itdependsnetworks
Copy link
Contributor

@jmcgill298 any chance you can take a look?

@jmcgill298
Copy link
Contributor

The references to _build_banner all store the value as a variable, and the docstrings document expected Returns:

Returns:
str: The next configuration line in the configuration text.
None: When banner end is the end of the config text.
(and replicated in other implementations)

The references to _build_nested_config all store the value as a variable, and the docstrings document expected Returns:

Returns:
str: The next top-level configuration line in the configuration text.
None: When the last line of configuration text is a nested configuration line.

The references to _build_multiline_config do not store it as a variable, so that method should remove returns and always return None.

The reference to _build_multiline_single_configuration_line does not store it as a variable, so that method should remove returns and always return None.

I suspect that the last two methods had a return to simplify writing test cases

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

No branches or pull requests

3 participants