Skip to content

Conversation

@mukulpalit-ibm
Copy link
Contributor

@mukulpalit-ibm mukulpalit-ibm commented Jul 14, 2025

Description

  • Improve DA's user experience
    Issue

The name app_config_cbr_rules to changes to cbr_rules to be consistent with other DAs . This change has been tested with the main branch and the plan only resulted in update-in-place and that too not because of the name change but because of the order of rule_contexts. Also verified in the UI that the CBR rule was not re created with the Rule ID.

Release required?

  • No release
  • Patch release (x.x.X)
  • Minor release (x.X.x)
  • Major release (X.x.x)
Release notes content

This PR Updates the catalog json for improved DA User experience as well as updates the IAM permissions needed to successfully deploy the DA with add ons. Also the variable name app_config_cbr_rules has been changed to cbr_rules

Run the pipeline

If the CI pipeline doesn't run when you create the PR, the PR requires a user with GitHub collaborators access to run the pipeline.

Run the CI pipeline when the PR is ready for review and you expect tests to pass. Add a comment to the PR with the following text:

/run pipeline

Checklist for reviewers

  • If relevant, a test for the change is included or updated with this PR.
  • If relevant, documentation for the change is included or updated with this PR.

For mergers

  • Use a conventional commit message to set the release level. Follow the guidelines.
  • Include information that users need to know about the PR in the commit message. The commit message becomes part of the GitHub release notes.
  • Use the Squash and merge option.

@mukulpalit-ibm
Copy link
Contributor Author

/run pipeline

Copy link
Member

@maheshwarishikha maheshwarishikha left a comment

Choose a reason for hiding this comment

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

  • I understand that you are making appconfig addons enabled, hence architecture diagram also needs to modify.

  • Please recheck the notes for the iam_permissions section in ibm_catalog.json. Does not look consistent the way it is written in others DA. I'll prefer to check those PRs (like ocp-ai) that is reviewed by content-reviewer.

  • One minor comment on features. Does it allow to create multiple collections or only one collection? Accordingly feature can be updated, if multiple collections are allowed.

@mukulpalit-ibm
Copy link
Contributor Author

/run pipeline

Copy link
Member

@MatthewLemmond MatthewLemmond left a comment

Choose a reason for hiding this comment

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

few minor points to clarify

@mukulpalit-ibm
Copy link
Contributor Author

/run pipeline

@mukulpalit-ibm
Copy link
Contributor Author

/run pipeline

@mukulpalit-ibm
Copy link
Contributor Author

/run pipeline

@mukulpalit-ibm
Copy link
Contributor Author

/run pipeline

@mukulpalit-ibm
Copy link
Contributor Author

/run pipeline

MatthewLemmond
MatthewLemmond previously approved these changes Jul 29, 2025
@mukulpalit-ibm mukulpalit-ibm dismissed maheshwarishikha’s stale review July 29, 2025 17:36

All changes done and approval done by Matt

Copy link
Contributor

@ocofaigh ocofaigh left a comment

Choose a reason for hiding this comment

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

New approach we are rolling out is to set enable_platform_metrics to false by default please

@mukulpalit-ibm mukulpalit-ibm requested a review from ocofaigh August 4, 2025 04:46
MatthewLemmond
MatthewLemmond previously approved these changes Aug 4, 2025
@mukulpalit-ibm
Copy link
Contributor Author

/run pipeline

Copy link
Member

@maheshwarishikha maheshwarishikha left a comment

Choose a reason for hiding this comment

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

  • enable_platform_metrics should be false by default
  • Please remove the work-around for virtual inputs as it has been fixed by catalog team.
  • As it is single service DA and as mentioned in instructions, we need to add some standard content with i icon. Thats missing, please refer instructions in the issue 13599.
  • Short description - Creates and configures an App Configuration service on IBM Cloud with optional integration of observability
  • As the service supports ca-mon region, do you need to add vpe support too in providers.tf?
  • DA-cbr_rules.md should have tag for account id and others as done here

@mukulpalit-ibm
Copy link
Contributor Author

  • enable_platform_metrics should be false by default
  • Please remove the work-around for virtual inputs as it has been fixed by catalog team.
  • As it is single service DA and as mentioned in instructions, we need to add some standard content with i icon. Thats missing, please refer instructions in the issue 13599.
  • Short description - Creates and configures an App Configuration service on IBM Cloud with optional integration of observability
  • As the service supports ca-mon region, do you need to add vpe support too in providers.tf?
  • DA-cbr_rules.md should have tag for account id and others as done here

done

@mukulpalit-ibm
Copy link
Contributor Author

/run pipeline

@mukulpalit-ibm
Copy link
Contributor Author

/run pipeline

@mukulpalit-ibm
Copy link
Contributor Author

/run pipeline

@mukulpalit-ibm
Copy link
Contributor Author

Screenshot 2025-08-12 at 11 37 12 AM Screenshot 2025-08-12 at 11 37 31 AM

Copy link
Member

@maheshwarishikha maheshwarishikha left a comment

Choose a reason for hiding this comment

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

LGTM

@maheshwarishikha maheshwarishikha dismissed ocofaigh’s stale review August 12, 2025 06:29

Changes made as requested

@maheshwarishikha maheshwarishikha merged commit c9058bc into main Aug 12, 2025
2 checks passed
@maheshwarishikha maheshwarishikha deleted the 14186-improve-user-experience branch August 12, 2025 06:36
@terraform-ibm-modules-ops
Copy link
Contributor

🎉 This PR is included in version 1.9.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants