Skip to content

Feature/654 add and use general matrix#860

Draft
ArBridgeman wants to merge 6 commits into
mainfrom
feature/654_add_and_use_general_matrix
Draft

Feature/654 add and use general matrix#860
ArBridgeman wants to merge 6 commits into
mainfrom
feature/654_add_and_use_general_matrix

Conversation

@ArBridgeman

Copy link
Copy Markdown
Collaborator

relates to #654

Checklist

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? -> need to do
  • update the User Guide & other documentation? -> need to do
  • 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?

@ArBridgeman ArBridgeman marked this pull request as draft June 9, 2026 12:32
Comment thread .github/workflows/matrix.yml Outdated
workflow_call:
inputs:
matrix_keys:
description: "Space-separated BaseConfig keys to include in the generated matrix output"

@ArBridgeman ArBridgeman Jun 9, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Should check if space separated is secure, reasonable

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.

Why not a JSON string (which then must be an array)?

Comment thread test/unit/nox/_ci_test.py

@pytest.fixture
def config(tmp_path) -> BaseConfig:
class Config(BaseConfig):

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@tomuben and @tkilias
We should discuss if this is an ok pattern for the ITDE.

So the two options I see are:

  1. As built in, one can extend the BaseConfig with additional fields. These should be typed & pydantic can validate the input before other operations are executed. I think that's a nice feature.
  2. We allow BaseConfig to have extra fields which are not checked by pydantic. The benefit would be that you do not have to inherit and extend the BaseConfig.

@@ -1,40 +0,0 @@
(( workflow_header ))

@ArBridgeman ArBridgeman Jun 9, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Similar to how we have released slow-checks.yml from PTB control, I though we could remove these template ymls. If a project cannot immediately migrate, they can retain these files. We will keep the nox sessions until 2026-09-15, so there isn't a major change. Does that sound alright or do we need to not do (or a major release)?

@ArBridgeman ArBridgeman temporarily deployed to manual-approval June 9, 2026 12:38 — with GitHub Actions Inactive
@ArBridgeman

ArBridgeman commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator Author
image

should ensure if that workflow is changed that it is run 😱

@ArBridgeman

Copy link
Copy Markdown
Collaborator Author
  • add/modify documentation

@ArBridgeman ArBridgeman deployed to manual-approval June 10, 2026 12:10 — with GitHub Actions Active
@sonarqubecloud

Copy link
Copy Markdown

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.

2 participants