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

ROX-10845: Refactor notifier scrubbing #1803

Merged
merged 2 commits into from May 31, 2022
Merged

Conversation

mtodor
Copy link
Contributor

@mtodor mtodor commented May 23, 2022

Description

We have moved logic for scrubbing of notifiers to the datastore and introduced pair interface functions with and without scrubbing.

NOTE: for more information, please check the related ticket.

Checklist

  • Investigated and inspected CI test results
  • Unit test and regression tests added
  • [ ] Evaluated and added CHANGELOG entry if required - not relevant
  • [ ] Determined and documented upgrade steps - not relevant
  • [ ] Documented user facing changes (create PR based on openshift/openshift-docs and merge into rhacs-docs) - not relevant

Testing Performed

  1. Login into UI as admin
  2. Create a test PagerDuty notifier
  3. Get Bearer token from Browser
export TOKEN='<my-bearer-token>'
  1. Run the curl command to get data over REST API
curl --request GET --insecure --url https://localhost:8000/v1/notifiers --header "Authorization: Bearer ${TOKEN}" --silent --output -
  1. Run the curl command to get data over GraphQL
curl --request POST --insecure --url https://localhost:8000/api/graphql --header "Authorization: Bearer ${TOKEN}" --header 'Content-Type: application/json' --data '{"query":"{notifiers{pagerduty{apiKey}}}"}' --silent --output -

@openshift-ci
Copy link

openshift-ci bot commented May 23, 2022

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@roxbot
Copy link
Contributor

roxbot commented May 23, 2022

Tag for build #608969 is 3.70.x-239-gcbf11648fc.

💻 For deploying this image using the dev scripts, run the following first:

export MAIN_IMAGE_TAG='3.70.x-239-gcbf11648fc'

🕹️ A roxctl binary can be downloaded from the CircleCI artifacts.

@mtodor mtodor marked this pull request as ready for review May 24, 2022 06:06
@mtodor
Copy link
Contributor Author

mtodor commented May 25, 2022

/test gke-qa-e2e-tests

@mtodor mtodor force-pushed the mtodor/ROX-10845-scrub-notifiers branch from b397396 to ba39331 Compare May 25, 2022 12:38
@mtodor mtodor force-pushed the mtodor/ROX-10845-scrub-notifiers branch from ba39331 to cbf1164 Compare May 30, 2022 08:25
@openshift-ci
Copy link

openshift-ci bot commented May 30, 2022

@mtodor: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/gke-qa-e2e-tests cbf1164 link false /test gke-qa-e2e-tests

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

Copy link
Contributor

@viswajithiii viswajithiii left a comment

Choose a reason for hiding this comment

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

Makes sense, thanks for doing this!

Only suggestion is to update the PR title (and, therefore, commit message) to be more specific about the change being done (something like "Move notifier scrubbing to datastore layer") rather than just saying "refactor".

@mtodor
Copy link
Contributor Author

mtodor commented May 31, 2022

/test style-checks

@mtodor mtodor merged commit c08f23f into master May 31, 2022
63 of 64 checks passed
@mtodor mtodor deleted the mtodor/ROX-10845-scrub-notifiers branch May 31, 2022 07:26
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.

None yet

3 participants