Skip to content

Split links away from tmt.base.core#4901

Open
happz wants to merge 1 commit into
mainfrom
base-split-links
Open

Split links away from tmt.base.core#4901
happz wants to merge 1 commit into
mainfrom
base-split-links

Conversation

@happz
Copy link
Copy Markdown
Contributor

@happz happz commented May 18, 2026

Pull Request Checklist

  • implement the feature

@happz happz added this to planning May 18, 2026
@happz happz added code | base Base classes implementation in base.py code | no functional change "No Functional Change" intended. Patch should not change tmt's behavior in any way. labels May 18, 2026
@github-project-automation github-project-automation Bot moved this to backlog in planning May 18, 2026
@happz happz added the ci | full test Pull request is ready for the full test execution label May 18, 2026
@happz happz moved this from backlog to review in planning May 18, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors link-related logic by moving Link, Links, and LinkNeedle classes from tmt.base.core to a new tmt.base.links module. Feedback identifies several missing runtime imports for Links and FmfId that will cause NameError exceptions in tmt/base/core.py and tmt/base/links.py. Additionally, update type hints in tmt/utils/jira.py to import directly from the new module instead of using re-exports.

Comment thread tmt/base/core.py

def _normalize_link(key_address: str, value: _RawLinks, logger: tmt.log.Logger) -> 'Links':
def _normalize_link(key_address: str, value: '_RawLinks', logger: tmt.log.Logger) -> 'Links':
return Links(data=value)
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.

high

Add a local import for Links. It is used here at runtime but is only imported within the TYPE_CHECKING block, which will result in a NameError.

Suggested change
return Links(data=value)
from tmt.base.links import Links
return Links(data=value)

Comment thread tmt/base/core.py

# Append link with appropriate relation
links = Links(data=list(cast(list[_RawLink], Story._opt('link', []))))
links = Links(data=list(cast(list['_RawLink'], Story._opt('link', []))))
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.

high

Import Links locally. It is not available in the runtime scope. Note that Test.create (not included in this diff) also requires this fix.

        from tmt.base.links import Links
        links = Links(data=list(cast(list['_RawLink'], Story._opt('link', []))))

Comment thread tmt/base/links.py
Comment on lines +216 to +238
def to_spec(self) -> _RawLinkRelation:
"""
Convert to a form suitable for saving in a specification file

No matter what the original specification was, every link will
generate the very same type of specification, the ``relation: target``
one.

Output of this method is fully compatible with specification, and when
given to :py:meth:`from_spec`, it shall create a ``Link`` instance
with the same properties as the original one.

[1] https://tmt.readthedocs.io/en/stable/spec/core.html#link
"""

spec: _RawLinkRelation = {
self.relation: self.target.to_spec() if isinstance(self.target, FmfId) else self.target
}

if self.note is not None:
spec['note'] = self.note

return spec
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.

high

Import FmfId locally within this method. It is used in the isinstance check but is currently only imported inside from_spec, leading to a NameError at runtime.

    def to_spec(self) -> _RawLinkRelation:
        """
        Convert to a form suitable for saving in a specification file

        No matter what the original specification was, every link will
        generate the very same type of specification, the relation: target
        one.

        Output of this method is fully compatible with specification, and when
        given to :py:meth:from_spec, it shall create a Link instance
        with the same properties as the original one.

        [1] https://tmt.readthedocs.io/en/stable/spec/core.html#link
        """

        from tmt.base.core import FmfId

        spec: _RawLinkRelation = {
            self.relation: self.target.to_spec() if isinstance(self.target, FmfId) else self.target
        }

        if self.note is not None:
            spec['note'] = self.note

        return spec

Comment thread tmt/utils/jira.py
if TYPE_CHECKING:
import jira

import tmt.base.links
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.

medium

Update type hints throughout this file to use Link from tmt.base.links instead of tmt.base.core (e.g., in add_link_to_issue and save_link_to_metadata).

References
  1. Avoid using init.py to re-export dependencies for sibling modules. Modules should import their dependencies directly.

@happz happz force-pushed the base-split-links branch from 5e8ca76 to 7939772 Compare May 19, 2026 15:14
@happz happz moved this from review to implement in planning May 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci | full test Pull request is ready for the full test execution code | base Base classes implementation in base.py code | no functional change "No Functional Change" intended. Patch should not change tmt's behavior in any way.

Projects

Status: implement

Development

Successfully merging this pull request may close these issues.

1 participant