Skip to content

#940 Added shared validation for packaged skills - #954

Open
jana-selva wants to merge 8 commits into
mainfrom
feature/940-shared-skill-validation
Open

jana-selva wants to merge 8 commits into
mainfrom
feature/940-shared-skill-validation

Conversation

@jana-selva

Copy link
Copy Markdown
Contributor

Checklist

Fixes #940
Note: If any of the items in the checklist are not relevant to your PR, just check the box.

For any Pull Request

Is the following correct:

  • the title of the Pull Request?
  • the title of the corresponding issue?
  • there are no other open Pull Requests for the same update/change?
  • that the issue which this Pull Request fixes ("Fixes...") is mentioned?

When Changes Were Made

Did you:

  • update the changelog?
  • update the cookiecutter-template?
  • update the implementation?
  • check coverage and add tests: unit tests and, if relevant, integration tests?
  • update the User Guide & other documentation?
  • resolve any failing CI criteria (incl. Sonar quality gate)?

When Preparing a Release

Have you:

  • thought about version number (major, minor, patch)?
  • checked Exasol packages for updates and resolved open vulnerabilities, if easily possible?

@jana-selva
jana-selva deployed to manual-approval September 15, 2026 07:51 — with GitHub Actions Active
@jana-selva
jana-selva deployed to manual-approval September 15, 2026 07:51 — with GitHub Actions Active
@jana-selva
jana-selva deployed to manual-approval September 15, 2026 09:29 — with GitHub Actions Active
@jana-selva
jana-selva deployed to manual-approval September 15, 2026 09:29 — with GitHub Actions Active
@jana-selva
jana-selva deployed to manual-approval September 15, 2026 09:34 — with GitHub Actions Active
@jana-selva
jana-selva deployed to manual-approval September 15, 2026 09:34 — with GitHub Actions Active
@jana-selva
jana-selva deployed to manual-approval September 15, 2026 09:47 — with GitHub Actions Active
@jana-selva
jana-selva deployed to manual-approval September 15, 2026 09:47 — with GitHub Actions Active
@jana-selva
jana-selva added this pull request to stack #961 September 18, 2026 05:45
Comment thread exasol/toolbox/nox/_skills.py
@jana-selva
jana-selva deployed to manual-approval September 18, 2026 09:42 — with GitHub Actions Active
@jana-selva
jana-selva deployed to manual-approval September 18, 2026 09:42 — with GitHub Actions Active
@sonarqubecloud

Copy link
Copy Markdown

@nox.session(name="skills:check", python=False)
def check_skills(session: Session) -> None:
"""Validate the common structure and content rules for packaged skills."""
failures = {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could we reduce 1 for-loop with something like:

failures = {}
  for skill_name in get_packaged_skill_names():
      errors = validate_skill(skill_name)
      if errors:
          failures[skill_name] = errors

  if failures:
      details = "\n".join(
          _format_skill_errors(skill_name, errors)
          for skill_name, errors in failures.items()
      )

@nox.session(name="skills:check", python=False)
def check_skills(session: Session) -> None:
"""Validate the common structure and content rules for packaged skills."""
failures = {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Would it be worthwhile in this entry point to check if the directory we'll check exists or not?

Currently, if someone runs this, the exception gets raised later. We tend to prefer failing earlier with a clear/controlled exception that indicates what to do.

❯ poetry run -- nox -s skills:check
nox > Running session skills:check
nox > Session skills:check raised exception FileNotFoundError(2, 'No such file or directory')
Traceback (most recent call last):
  File "/home/arsc/.cache/pypoetry/virtualenvs/exasol-toolbox-HhsxY0p2-py3.10/lib/python3.10/site-packages/nox/sessions.py", line 1187, in execute
    self.func(session)
  File "/home/arsc/.cache/pypoetry/virtualenvs/exasol-toolbox-HhsxY0p2-py3.10/lib/python3.10/site-packages/nox/_decorators.py", line 100, in __call__
    return self.func(*args, **kwargs)
  File "/home/arsc/Code/python-toolbox/exasol/toolbox/nox/_skills.py", line 16, in check_skills
    for skill_name in get_packaged_skill_names()
  File "/home/arsc/Code/python-toolbox/exasol/toolbox/util/skills.py", line 47, in get_packaged_skill_names
    return tuple(sorted(path.name for path in skills_path.iterdir() if path.is_dir()))
  File "/home/arsc/Code/python-toolbox/exasol/toolbox/util/skills.py", line 47, in <genexpr>
    return tuple(sorted(path.name for path in skills_path.iterdir() if path.is_dir()))
  File "/home/arsc/.local/share/pypoetry/python/cpython@3.10.18/lib/python3.10/pathlib.py", line 1017, in iterdir
    for name in self._accessor.listdir(self):
FileNotFoundError: [Errno 2] No such file or directory: "MultiplexedPath('/home/arsc/Code/python-toolbox/exasol/toolbox/skills')"
nox > Session skills:check failed.

)

with pytest.raises(_SessionQuit, match="Packaged skill validation failed"):
_skills.check_skills(nox_session)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Would you want to inspect the caught exception to see that the different errors were present in it?
I'm asking as the test name sounds like that is the intent.

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.

Add shared validation for PTB skills

3 participants