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

RHIROS-1341 Fix sorting for workload type #139

Closed
wants to merge 2 commits into from

Conversation

srdeotarse
Copy link
Contributor

@srdeotarse srdeotarse commented Oct 19, 2023

Added new migration to alter workloadtype type of column workloads.workload_type to correct order of constants in workloadtype enum

Also removed make local-upload-data and updated make get-recommendations as suggested by @patilsuraj767

upload-msg-to-rosocp:
echo ${ros_ocp_msg} | docker-compose -f scripts/docker-compose.yml exec -T kafka kafka-console-producer --topic hccm.ros.events --broker-list localhost:29092


get-recommendations:
ifdef env
$(eval APIPOD=$(shell oc get pods -o custom-columns=POD:.metadata.name --no-headers -n ${env} | grep ros-ocp-backend-api))
oc exec ${APIPOD} -c ros-ocp-backend-api -n ${env} -- /bin/bash -c 'curl -s -H "X-Rh-Identity: ${b64_identity}" -H "x-rh-request_id: testtesttest" http://localhost:8000/api/cost-management/v1/recommendations/openshift' | python -m json.tool
oc exec ${APIPOD} -c ros-ocp-backend-api -n ${env} -- /bin/bash -c 'curl -s -H "X-Rh-Identity: ${b64_identity}" -H "x-rh-request_id: testtesttest" http://localhost:8000/api/cost-management/v1/recommendations/openshift?start_date=2023-04-01' | python -m json.tool
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to not hardcode the start_date? Its my assumption that make file should have a way to pass cmd line arguments and then those can be used?

something like:

make get-recommendations start_date=2023-08-01

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The ros-ocp-usage.csv file we are using to upload data to local kafka topic for processor to upload results to recommendations populated after this date. So, I had faced an issue as a newcomer that I was not able to fetch recommendations because I didn't knew what start date was.

Thanks @upadhyeammit for pointing this out. Maybe @patilsuraj767 has something to add to this?

Copy link
Contributor

Choose a reason for hiding this comment

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

@upadhyeammit Feel free to continue, #140 (comment)

else
curl -s -H "x-rh-identity: ${b64_identity}" \
-H "x-rh-request_id: testtesttest" \
http://localhost:8000/api/cost-management/v1/recommendations/openshift | python -m json.tool
http://localhost:8000/api/cost-management/v1/recommendations/openshift?start_date=2023-04-01 | python -m json.tool
Copy link
Contributor

Choose a reason for hiding this comment

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

Here as well if we can avoid the hardcoding of date?

@@ -0,0 +1,9 @@
ALTER TYPE workloadtype RENAME TO workloadtype_old;
Copy link
Contributor

Choose a reason for hiding this comment

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

I was under impression that if we rename the ENUM then table won't be able to reference it or we need to manually change the reference however I can see its updating the name in table as well. So from migration point of view it looks good to me!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have followed this article for reference.

@saltgen
Copy link
Contributor

saltgen commented Oct 20, 2023

@srdeotarse Any luck with the PR check failure?

@srdeotarse srdeotarse closed this Oct 23, 2023
@srdeotarse srdeotarse reopened this Oct 23, 2023
@srdeotarse
Copy link
Contributor Author

Closing this PR as opened new PR - #140 from forked repo

@srdeotarse srdeotarse closed this Oct 23, 2023
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.

3 participants