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

Refactor Item #3439

Open
SchoolGuy opened this issue May 31, 2023 · 7 comments · May be fixed by #3440
Open

Refactor Item #3439

SchoolGuy opened this issue May 31, 2023 · 7 comments · May be fixed by #3440
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@SchoolGuy
Copy link
Member

Is your feature request related to a problem?

As a developer
I want to implement the models that my users use correctly
so that I can provide the necessary features.

Provide a detailed description of the proposed feature

Currently, all Items in Cobbler are assumed to be inheriting from a tree of parent objects. In reality, not all of our models have logical parents that require this. Since the parent logic is a costly operation we should try to make the functionality of it available to items that don't require inheritance.

If we do this in the right way this would also slim down our data structures. Not all items require to have kernel_options or autoinstall_meta dictionaries. As all of these data structures need to be set during serialization and deserialization and this takes CPU cycles that I don't want to spend. Smaller data structures also should mean that there are less errors since there is less room to make those errors.

Alternatives you've considered

None

Additional information

This is a braindump by me. Which should act as a tracker for the corresponding PR.

@SchoolGuy SchoolGuy added the enhancement New feature or request label May 31, 2023
@SchoolGuy SchoolGuy added this to the v3.4.0 milestone May 31, 2023
@SchoolGuy SchoolGuy self-assigned this May 31, 2023
@SchoolGuy SchoolGuy linked a pull request May 31, 2023 that will close this issue
12 tasks
@SchoolGuy
Copy link
Member Author

Yesterday and today I invested a large chunk of my day into the documentation of the Cobbler data structures into UML so that the reorganization can be easier done on paper before implementation. The linked draft PR showed that moving the structures around is not as easy as it appeared before.

@SchoolGuy
Copy link
Member Author

My investigation continued today. I will upload the DrawIO diagram as soon as I have a complete first draft available. Atm the document is still under active development.

@SchoolGuy
Copy link
Member Author

SchoolGuy commented Dec 31, 2023

The plan I think for this is the following:

  1. Split out the item cache into a dedicated module. - Items: Move ItemCache into dedicated submodule #3535
  2. Have a green test suite with the refactoring.
  3. Split out network interfaces as a dedicated item.
  4. Have a green test suite with the new item type.
  5. Split the base Item out as a new abstract item type. (Leave Item as is with inheritance.)
  6. Have a green test suite with the new item type.
  7. Split Item into subclasses.
  8. Have a green test suite with the new item type and the refactored existing item types.

@SchoolGuy
Copy link
Member Author

Okay since GitHub doesn't support diagrams directly in GitHub comments, I have to upload it to a GitHub Gist and post the screenshot here.

@SchoolGuy
Copy link
Member Author

SchoolGuy commented Dec 31, 2023

Current state:

Cobbler Inheritance-Ist Zustand drawio

Desired To-Be state:

Cobbler Inheritance-Soll Zustand drawio

Gist with drawio diagram: https://gist.github.com/SchoolGuy/4f7da2f292e2ba0fd27bf5bbf5ce5d33

The Gist will be continuously updated as the diagram changes (if it changes).

@SchoolGuy
Copy link
Member Author

The outcome of #3416 (comment) shows that we can remove the tree about configuration management. This will make our main refactoring much easier.

@SchoolGuy
Copy link
Member Author

I updated the Gist since the Config Management is removed now.

Cobbler Inheritance-Soll-Zustand V2 drawio

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: In Progress
Development

Successfully merging a pull request may close this issue.

1 participant