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

refactor: get platform calls #1251

Merged
merged 2 commits into from
Sep 20, 2024

Conversation

zdtsw
Copy link
Member

@zdtsw zdtsw commented Sep 19, 2024

Description

How Has This Been Tested?

local build: quay.io/wenzhou/opendatahub-operator:2.18.919-1

Screenshot or short clip

Merge criteria

  • You have read the contributors guide.
  • Commit messages are meaningful - have a clear and concise summary and detailed explanation of what was changed and why.
  • Pull Request contains a description of the solution, a link to the JIRA issue, and to any dependent or related Pull Request.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

- move check on platform into top of dsc and dsci reconcile to reduce duplicated calls
- add logger when operator start to show which platform is running on

Signed-off-by: Wen Zhou <[email protected]>
@openshift-ci openshift-ci bot requested review from gzaronikas and lphiri September 19, 2024 12:22
@zdtsw
Copy link
Member Author

zdtsw commented Sep 19, 2024

@VaishnaviHire
Copy link
Member

Given this change, should we add an explicit testcase in e2e that verifies platform?

@zdtsw
Copy link
Member Author

zdtsw commented Sep 19, 2024

Given this change, should we add an explicit testcase in e2e that verifies platform?

i can add a simple one

@openshift-ci openshift-ci bot removed the lgtm label Sep 20, 2024
@zdtsw
Copy link
Member Author

zdtsw commented Sep 20, 2024

Given this change, should we add an explicit testcase in e2e that verifies platform?

i can add a simple one

updated 4 unit-tests for each case.

@openshift-ci openshift-ci bot added the lgtm label Sep 20, 2024
Copy link

openshift-ci bot commented Sep 20, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ykaliuta

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@zdtsw zdtsw requested review from VaishnaviHire and removed request for lphiri and gzaronikas September 20, 2024 12:22
@zdtsw zdtsw merged commit d73b3da into opendatahub-io:incubation Sep 20, 2024
7 of 8 checks passed
ykaliuta added a commit to ykaliuta/opendatahub-operator that referenced this pull request Sep 23, 2024
There is no need to maintain the api since GetRelease performs the
call and returns the information.

The extra work GetRelease does (in addition to GetPlatform) is not
important since only one call in startup left.

Since the api removed, tests do not make sense, so revert testing
part of d73b3da ("refactor: get platform calls (opendatahub-io#1251)")

Signed-off-by: Yauheni Kaliuta <[email protected]>
@ykaliuta ykaliuta mentioned this pull request Sep 23, 2024
5 tasks
openshift-merge-bot bot pushed a commit that referenced this pull request Sep 23, 2024
There is no need to maintain the api since GetRelease performs the
call and returns the information.

The extra work GetRelease does (in addition to GetPlatform) is not
important since only one call in startup left.

Since the api removed, tests do not make sense, so revert testing
part of d73b3da ("refactor: get platform calls (#1251)")

Signed-off-by: Yauheni Kaliuta <[email protected]>
zdtsw referenced this pull request in zdtsw-forking/rhods-operator Oct 10, 2024
* refactor: get platform calls

- move check on platform into top of dsc and dsci reconcile to reduce duplicated calls
- add logger when operator start to show which platform is running on
- test: add testcase for GetPlatform()
---------

Signed-off-by: Wen Zhou <[email protected]>
(cherry picked from commit d73b3da)
zdtsw referenced this pull request in zdtsw-forking/rhods-operator Oct 10, 2024
There is no need to maintain the api since GetRelease performs the
call and returns the information.

The extra work GetRelease does (in addition to GetPlatform) is not
important since only one call in startup left.

Since the api removed, tests do not make sense, so revert testing
part of d73b3da ("refactor: get platform calls (red-hat-data-services#1251)")

Signed-off-by: Yauheni Kaliuta <[email protected]>
(cherry picked from commit d6eda01)
zdtsw referenced this pull request in zdtsw-forking/rhods-operator Oct 10, 2024
* refactor: get platform calls

- move check on platform into top of dsc and dsci reconcile to reduce duplicated calls
- add logger when operator start to show which platform is running on
- test: add testcase for GetPlatform()
---------

Signed-off-by: Wen Zhou <[email protected]>
(cherry picked from commit d73b3da)
zdtsw referenced this pull request in zdtsw-forking/rhods-operator Oct 10, 2024
There is no need to maintain the api since GetRelease performs the
call and returns the information.

The extra work GetRelease does (in addition to GetPlatform) is not
important since only one call in startup left.

Since the api removed, tests do not make sense, so revert testing
part of d73b3da ("refactor: get platform calls (red-hat-data-services#1251)")

Signed-off-by: Yauheni Kaliuta <[email protected]>
(cherry picked from commit d6eda01)
zdtsw referenced this pull request in zdtsw-forking/rhods-operator Oct 17, 2024
* refactor: get platform calls

- move check on platform into top of dsc and dsci reconcile to reduce duplicated calls
- add logger when operator start to show which platform is running on
- test: add testcase for GetPlatform()
---------

Signed-off-by: Wen Zhou <[email protected]>
(cherry picked from commit d73b3da)
zdtsw referenced this pull request in zdtsw-forking/rhods-operator Oct 17, 2024
There is no need to maintain the api since GetRelease performs the
call and returns the information.

The extra work GetRelease does (in addition to GetPlatform) is not
important since only one call in startup left.

Since the api removed, tests do not make sense, so revert testing
part of d73b3da ("refactor: get platform calls (red-hat-data-services#1251)")

Signed-off-by: Yauheni Kaliuta <[email protected]>
(cherry picked from commit d6eda01)
zdtsw referenced this pull request in zdtsw-forking/rhods-operator Oct 17, 2024
* refactor: get platform calls

- move check on platform into top of dsc and dsci reconcile to reduce duplicated calls
- add logger when operator start to show which platform is running on
- test: add testcase for GetPlatform()
---------

Signed-off-by: Wen Zhou <[email protected]>
(cherry picked from commit d73b3da)
zdtsw referenced this pull request in zdtsw-forking/rhods-operator Oct 17, 2024
There is no need to maintain the api since GetRelease performs the
call and returns the information.

The extra work GetRelease does (in addition to GetPlatform) is not
important since only one call in startup left.

Since the api removed, tests do not make sense, so revert testing
part of d73b3da ("refactor: get platform calls (red-hat-data-services#1251)")

Signed-off-by: Yauheni Kaliuta <[email protected]>
(cherry picked from commit d6eda01)
openshift-merge-bot bot referenced this pull request in red-hat-data-services/rhods-operator Oct 17, 2024
* refactor: get platform calls

- move check on platform into top of dsc and dsci reconcile to reduce duplicated calls
- add logger when operator start to show which platform is running on
- test: add testcase for GetPlatform()
---------

Signed-off-by: Wen Zhou <[email protected]>
(cherry picked from commit d73b3da)
openshift-merge-bot bot referenced this pull request in red-hat-data-services/rhods-operator Oct 17, 2024
There is no need to maintain the api since GetRelease performs the
call and returns the information.

The extra work GetRelease does (in addition to GetPlatform) is not
important since only one call in startup left.

Since the api removed, tests do not make sense, so revert testing
part of d73b3da ("refactor: get platform calls (#1251)")

Signed-off-by: Yauheni Kaliuta <[email protected]>
(cherry picked from commit d6eda01)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants