Skip to content

refactor: modularize dynamic module and port class creation logic - #180

Open
bhagathkrishnacdac wants to merge 1 commit into
omec-project:mainfrom
bhagathkrishnacdac:bess-refactor-commands-functions
Open

refactor: modularize dynamic module and port class creation logic#180
bhagathkrishnacdac wants to merge 1 commit into
omec-project:mainfrom
bhagathkrishnacdac:bess-refactor-commands-functions

Conversation

@bhagathkrishnacdac

Copy link
Copy Markdown

Description
This PR refactors _get_bess_module_and_port_creators to address cognitive complexity. The original monolithic function, which managed BESS RPC lookups, namespace collision checks, exception formatting, and dynamic Python type generation, has been decomposed into small, single-responsibility helper functions.

Key Changes

  • Separated Name Collection: Extracted _collect_module_and_driver_names to query the active driver and module class names from the BESS daemon.
  • Isolated Namespace Collision Checks:
    • _find_duplicate_names evaluates naming collisions between reserved words, modules, and drivers using collections.Counter.
    • _generate_duplicate_error_message formats descriptive error strings if an invalid namespace overlap is detected.
  • Decoupled Type Instantiation: Extracted _create_module_creators and _create_port_creators to dynamically construct subclass objects of Module and Port using the standard type() constructor.

Signed-off-by: bhagathkrishnacdac <bhagath.krishna@cdac.in>

@gab-arrobo gab-arrobo left a comment

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.

@bhagathkrishnacdac,
Thank you for your contribution. I see multiple "trailing whitespace" warning when trying to apply your patch locally. This seems to happen across the multiple contributions/PRs. Thank you!

pr.patch:70: trailing whitespace.
def _collect_module_and_driver_names(cli):
pr.patch:71: trailing whitespace.
    """Collect module class names and port driver names from BESS."""
pr.patch:72: trailing whitespace.
    class_names = [str(i) for i in cli.bess.list_mclasses().names]
pr.patch:73: trailing whitespace.
    driver_names = [str(i) for i in cli.bess.list_drivers().driver_names]
pr.patch:76: trailing whitespace.
def _find_duplicate_names(rsvd, class_names, driver_names):
warning: squelched 67 whitespace errors
warning: 72 lines add whitespace errors.
``

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Refactors bessctl/commands.py to reduce cognitive complexity in _get_bess_module_and_port_creators by decomposing BESS RPC name collection, namespace-collision detection, and dynamic type construction into focused helper functions.

Changes:

  • Extracted helper to collect module class names and port driver names from the BESS daemon.
  • Isolated duplicate-name detection and formatting into dedicated helpers.
  • Split dynamic type() creator construction into separate module/port creator helpers.
Suppressed comments (1)

bessctl/commands.py:792

  • The refactor removed the in-code rationale around why duplicate names are considered a BESS fault and what invariants the C++ layer already enforces. Keeping that context near the duplicate-detection logic will help future maintainers avoid weakening the constraints unintentionally.
    """Find duplicate names between reserved names, modules, and drivers."""  
    counts = collections.Counter(rsvd.keys())  
    counts.update(class_names)  
    counts.update(driver_names)  

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread bessctl/commands.py
Comment on lines +782 to +786
def _collect_module_and_driver_names(cli):
"""Collect module class names and port driver names from BESS."""
class_names = [str(i) for i in cli.bess.list_mclasses().names]
driver_names = [str(i) for i in cli.bess.list_drivers().driver_names]
return class_names, driver_names
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