Skip to content

[Fix] changes of Jira variable according to new migrated jira#1257

Merged
dbasunag merged 4 commits intoopendatahub-io:mainfrom
mwaykole:jirafix1
Mar 20, 2026
Merged

[Fix] changes of Jira variable according to new migrated jira#1257
dbasunag merged 4 commits intoopendatahub-io:mainfrom
mwaykole:jirafix1

Conversation

@mwaykole
Copy link
Copy Markdown
Member

@mwaykole mwaykole commented Mar 19, 2026

Pull Request

Summary

Related Issues

  • Fixes:
  • JIRA:

How it has been tested

  • Locally
  • Jenkins

Additional Requirements

  • If this PR introduces a new test image, did you create a PR to mirror it in disconnected environment?
  • If this PR introduces new marker(s)/adds a new component, was relevant ticket created to update relevant Jenkins job?

Summary by CodeRabbit

  • Documentation

    • Updated Jira integration setup documentation with new authentication requirements.
  • Chores

    • Updated Jira authentication to use username and password credentials instead of token-based authentication.

Signed-off-by: Milind waykole <mwaykole@redhat.com>
@mwaykole mwaykole requested a review from a team as a code owner March 19, 2026 10:08
@mwaykole mwaykole requested a review from dbasunag March 19, 2026 10:09
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 19, 2026

📝 Walkthrough

Walkthrough

Authentication mechanism for Jira connection changed from token-based to username/password-based. A pytest hook migrates PYTEST_JIRA_TOKEN environment variable to PYTEST_JIRA_PASSWORD during test startup, and Jira client authentication parameters were updated accordingly. Documentation reflects the new three-variable authentication requirement.

Changes

Cohort / File(s) Summary
Pytest Configuration Hook
conftest.py
Added pytest_configure hook that conditionally copies PYTEST_JIRA_TOKEN to PYTEST_JIRA_PASSWORD and removes the token env var if password not already set. Alters credential environment state before test execution.
Jira Authentication
utilities/jira.py
Modified get_jira_connection() basic_auth tuple from (PYTEST_JIRA_USERNAME, PYTEST_JIRA_TOKEN) to (PYTEST_JIRA_USERNAME, PYTEST_JIRA_PASSWORD).
Documentation
docs/GETTING_STARTED.md
Updated Jira integration requirements to document three mandatory environment variables: PYTEST_JIRA_URL, PYTEST_JIRA_USERNAME, and PYTEST_JIRA_TOKEN.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request description is almost entirely empty with only template placeholders and no substantive content filled in. Provide a concrete summary of the Jira authentication migration changes, fill in related JIRA ticket information, and document how the changes were tested locally.
Title check ❓ Inconclusive The title refers to Jira variable changes but is vague about what was actually changed and why (e.g., 'according to new migrated jira' is unclear). Clarify the title to specify the actual change: e.g., 'Use password-based auth instead of token for Jira integration' or similar actionable description.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

You can get early access to new features in CodeRabbit.

Enable the early_access setting to enable early access features such as new models, tools, and more.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
docs/GETTING_STARTED.md (1)

141-142: Consider documenting the token-to-password migration.

Users set PYTEST_JIRA_TOKEN per this doc, but utilities/jira.py reads PYTEST_JIRA_PASSWORD. The runtime migration in conftest.py bridges this, but that indirection isn't explained here. A brief note would prevent confusion when debugging authentication issues.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/GETTING_STARTED.md` around lines 141 - 142, Update the
GETTING_STARTED.md Jira section to note the token->password env var migration:
explain that while the doc instructs setting PYTEST_JIRA_TOKEN, the runtime code
in conftest.py will copy that value into PYTEST_JIRA_PASSWORD for backward
compatibility and utilities/jira.py reads PYTEST_JIRA_PASSWORD; explicitly
mention both variable names and that conftest.py performs the migration so users
know why either name may appear and how to troubleshoot authentication issues.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@utilities/jira.py`:
- Around line 25-28: The JIRA client creation uses os.getenv() which can return
None and lead to unclear failures; update the code that returns JIRA(...) (the
JIRA constructor call) to explicitly validate required environment variables
PYTEST_JIRA_URL, PYTEST_JIRA_USERNAME, and PYTEST_JIRA_PASSWORD before
constructing the client, and if any are missing raise a clear exception (e.g.,
ValueError) or log an explicit error mentioning which variables are absent so
callers fail fast with a helpful message rather than passing None into JIRA.

---

Nitpick comments:
In `@docs/GETTING_STARTED.md`:
- Around line 141-142: Update the GETTING_STARTED.md Jira section to note the
token->password env var migration: explain that while the doc instructs setting
PYTEST_JIRA_TOKEN, the runtime code in conftest.py will copy that value into
PYTEST_JIRA_PASSWORD for backward compatibility and utilities/jira.py reads
PYTEST_JIRA_PASSWORD; explicitly mention both variable names and that
conftest.py performs the migration so users know why either name may appear and
how to troubleshoot authentication issues.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 5202c436-749b-4cb7-a154-a7b9be817b05

📥 Commits

Reviewing files that changed from the base of the PR and between cc43a76 and 2e79808.

📒 Files selected for processing (3)
  • conftest.py
  • docs/GETTING_STARTED.md
  • utilities/jira.py

@github-actions
Copy link
Copy Markdown

The following are automatically added/executed:

  • PR size label.
  • Run pre-commit
  • Run tox
  • Add PR author as the PR assignee
  • Build image based on the PR

Available user actions:

  • To mark a PR as WIP, add /wip in a comment. To remove it from the PR comment /wip cancel to the PR.
  • To block merging of a PR, add /hold in a comment. To un-block merging of PR comment /hold cancel.
  • To mark a PR as approved, add /lgtm in a comment. To remove, add /lgtm cancel.
    lgtm label removed on each new commit push.
  • To mark PR as verified comment /verified to the PR, to un-verify comment /verified cancel to the PR.
    verified label removed on each new commit push.
  • To Cherry-pick a merged PR /cherry-pick <target_branch_name> to the PR. If <target_branch_name> is valid,
    and the current PR is merged, a cherry-picked PR would be created and linked to the current PR.
  • To build and push image to quay, add /build-push-pr-image in a comment. This would create an image with tag
    pr-<pr_number> to quay repository. This image tag, however would be deleted on PR merge or close action.
Supported labels

{'/cherry-pick', '/wip', '/build-push-pr-image', '/hold', '/verified', '/lgtm'}

fege
fege previously approved these changes Mar 19, 2026
Copy link
Copy Markdown
Contributor

@fege fege left a comment

Choose a reason for hiding this comment

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

/lgtm

@mwaykole
Copy link
Copy Markdown
Member Author

/build-and-push-image

@mwaykole
Copy link
Copy Markdown
Member Author

/build-and-push-image pr1255

@dbasunag
Copy link
Copy Markdown
Collaborator

/build-push-pr-image

@github-actions
Copy link
Copy Markdown

Status of building tag pr-1257: success.
Status of pushing tag pr-1257 to image registry: success.

Copy link
Copy Markdown
Collaborator

@dbasunag dbasunag left a comment

Choose a reason for hiding this comment

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

We need to check if we need this change. With #1251 merged, I am no longer seeing the 403 jira issues.

@mwaykole mwaykole closed this Mar 19, 2026
@mwaykole mwaykole reopened this Mar 19, 2026
@dbasunag
Copy link
Copy Markdown
Collaborator

I will push my changes against this branch soon. Adding a hold label to indicate that I am working on it.

Signed-off-by: Debarati Basu-Nag <dbasunag@redhat.com>
Copy link
Copy Markdown
Contributor

@fege fege left a comment

Choose a reason for hiding this comment

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

/lgtm

@dbasunag dbasunag merged commit 6bdb5ba into opendatahub-io:main Mar 20, 2026
10 checks passed
@github-actions
Copy link
Copy Markdown

Status of building tag latest: success.
Status of pushing tag latest to image registry: success.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants