Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added possibility to search for debian errata #1013

Merged
merged 1 commit into from
Dec 13, 2023

Conversation

dosas
Copy link
Collaborator

@dosas dosas commented Oct 23, 2023

in the form DSA-1234-1

@omkarkhatavkar
Copy link

Can one of the admins verify this patch?

@@ -40,7 +40,7 @@ def search(self, query, applicable=True, installable=False, repo=None):
if repo is not None:
self.repo_filter.fill(repo)

if re.search(r'\w{3,4}[:-]\d{4}[-:]\d{4}', query):
if re.search(r'\w{3,4}[:-]\d{4}[-:]\d{1,4}', query):

Choose a reason for hiding this comment

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

@sambible @vsedmik @vijaysawant Do you see any value for this regex? As per I know we never use the Debian errata, mostly test RHEL errata.

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't test any Debian errata that I'm aware of. This will change this particular regex to allow for
DSA-1234-1 where in the past it would not have matched, since RHEL errata are in the form
RHSA-1234-1234 ( With explicitly 4 characters in the last third)

These will still get captured by this adjusted regex, although it won't match them as strictly. If there's value to @dosas here, I'd be curious to know the use case

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My use case is to read and install debian errata with robottelo which currently is not possible since the search is a requirement for the ErrataDetailsView

Copy link
Collaborator Author

@dosas dosas Oct 25, 2023

Choose a reason for hiding this comment

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

Upon writing the above comment I realized that the same regexp is also used for this view .

I pushed a fix that will store the information in one place.

@dosas dosas force-pushed the feature/debian-errata branch from 1d853c9 to 6ea95f0 Compare October 25, 2023 13:31
@omkarkhatavkar
Copy link

@dosas We require additional time to thoroughly test this. I will need to conduct additional PRT tests to ensure that no issues arise. Since you are not an organization user, you do not have the ability to trigger the PRT tool. We are interested in exploring alternative testing methods that do not rely on permissions.

@JacobCallahan
Copy link
Member

@omkarkhatavkar I don't think PRT will help us here, aside from basic regression testing, since debian isn't supported by Satellite. This would need to be tested in an upstream build.

@omkarkhatavkar
Copy link

@JacobCallahan As you said rightly, running PRT to ensure regression is passing and we will have to ask the anyone from the Phonix team to test this with an upstream build. I guess @sambible @vsedmik @ColeHiggins2 could help.

@dosas
Copy link
Collaborator Author

dosas commented Dec 6, 2023

any news on this?

@sambible sambible added CherryPick PR needs CherryPick to previous branches 6.13.z 6.14.z 6.15.z labels Dec 8, 2023
@vsedmik
Copy link
Contributor

vsedmik commented Dec 8, 2023

trigger: test-robottelo
pytest: tests/foreman/ui/test_errata.py -k test_end_to_end

@sambible
Copy link
Contributor

sambible commented Dec 8, 2023

@dosas we are going to run some tests in PRT here, and if we see passes then I'm good with approving this request

@omkarkhatavkar
Copy link

trigger: test-robottelo
pytest: tests/foreman/ui/test_errata.py -k test_end_to_end

@dosas
Copy link
Collaborator Author

dosas commented Dec 13, 2023

I see that some tests are failing. Let me know if this is related to the change and I should fix something.

@omkarkhatavkar
Copy link

@vsedmik tests are failing not related to his PR. @dosas thanks for showing patience. will be merging this PR. Thanks for your contribution @dosas.

@omkarkhatavkar omkarkhatavkar merged commit 6cef8cc into SatelliteQE:master Dec 13, 2023
9 of 11 checks passed
github-actions bot pushed a commit that referenced this pull request Dec 13, 2023
in the form DSA-1234-1

(cherry picked from commit 6cef8cc)
@dosas
Copy link
Collaborator Author

dosas commented Dec 13, 2023

@vsedmik I saw that you removed cherry pick labels for 6.14 and 6.13 is there any strong reason for this. Would it be possible to cherry pick it into those branches?

@vsedmik
Copy link
Contributor

vsedmik commented Dec 14, 2023

@dosas oh no, it was a mid-air collision. I just added 6.15 and Sam added 6.15, 6.14, 6.13 at the same time, which rendered as me removing the diff :(

omkarkhatavkar pushed a commit that referenced this pull request Dec 19, 2023
in the form DSA-1234-1

(cherry picked from commit 6cef8cc)

Co-authored-by: dosas <[email protected]>
@dosas
Copy link
Collaborator Author

dosas commented Jan 8, 2024

@vsedmik what would we have to do to get these changes to 6.13.z and 6.14.z

damoore044 pushed a commit to damoore044/airgun that referenced this pull request Jan 12, 2024
vsedmik pushed a commit to vsedmik/airgun that referenced this pull request Jan 17, 2024
vsedmik pushed a commit to vsedmik/airgun that referenced this pull request Jan 17, 2024
@vsedmik
Copy link
Contributor

vsedmik commented Jan 17, 2024

@dosas I've opened manual cherry-pick PRs with your changes and triggered the PRT.
@sambible please review and merge if all ok.

sambible pushed a commit that referenced this pull request Jan 18, 2024
sambible pushed a commit that referenced this pull request Jan 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.15.z CherryPick PR needs CherryPick to previous branches
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants