Skip to content

456: catalog update nox task descriptions#895

Open
jana-selva wants to merge 6 commits into
mainfrom
doc/456-catalog-update-nox-task-descriptions
Open

456: catalog update nox task descriptions#895
jana-selva wants to merge 6 commits into
mainfrom
doc/456-catalog-update-nox-task-descriptions

Conversation

@jana-selva

@jana-selva jana-selva commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Fixes #456

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?
  • 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 force-pushed the doc/456-catalog-update-nox-task-descriptions branch from 85792d5 to b4b053c Compare June 23, 2026 11:01
@jana-selva jana-selva temporarily deployed to manual-approval June 23, 2026 11:01 — with GitHub Actions Inactive
@jana-selva jana-selva temporarily deployed to manual-approval June 23, 2026 11:01 — with GitHub Actions Inactive
@ArBridgeman

ArBridgeman commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

Comment thread doc/_includes/nox_sessions_catalog.rst Outdated
Comment thread doc/_includes/nox_sessions_catalog.rst Outdated
Comment thread doc/changes/unreleased.md Outdated
Comment on lines +91 to +97
with patch(
"exasol.toolbox.tools.security.gh_security_issues",
return_value=(),
):
result = cli_runner.invoke(
CVE_CLI, ["filter", Filter.GitHubIssues.value, str(json_path)]
)

@jana-selva jana-selva Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

after removing GITHUB_TOKEN, the CI failed, so i had to add with block to fix on that.. is this right or should i think of different approach ?

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 did you remove GITHUB_TOKEN in the first place?

I think this test was designed as an integration test and should not patch the access to GitHub.
It should also not patch the access to GitHub when there is no GitHub Token.

The fix is rather to ensure having a GitHub token.

I propose to re-add GitHub token to file slow-checks.yml.

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.

Yeah, I had suggested removing it. It's an artifact in many of the repositories but not in this case, it appears.

Comment on lines +91 to +97
with patch(
"exasol.toolbox.tools.security.gh_security_issues",
return_value=(),
):
result = cli_runner.invoke(
CVE_CLI, ["filter", Filter.GitHubIssues.value, str(json_path)]
)

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 did you remove GITHUB_TOKEN in the first place?

I think this test was designed as an integration test and should not patch the access to GitHub.
It should also not patch the access to GitHub when there is no GitHub Token.

The fix is rather to ensure having a GitHub token.

I propose to re-add GitHub token to file slow-checks.yml.

Comment thread .github/workflows/slow-checks.yml
@ckunki

ckunki commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

@ArBridgeman wrote

It turned our this var is required for at least some of the integration tests, in particular security_integration_test.py.

@jana-selva

jana-selva commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

I reverted that part: GITHUB_TOKEN is re-added in slow-checks.yml, and the mock-based test workaround was removed. The integration test depends on real gh access, so keeping the token is the better fit here.

@sonarqubecloud

Copy link
Copy Markdown

@jana-selva jana-selva deployed to manual-approval June 25, 2026 07:19 — with GitHub Actions Active
@jana-selva jana-selva temporarily deployed to manual-approval June 25, 2026 07:19 — with GitHub Actions Inactive
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.

Catalog & update descriptions of nox tasks in user & developer guides

3 participants