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

fix: remove watsonx mark as sensitive var #201

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kierramarie
Copy link

Description

Mark as sensitive var was add to Watsonx DA so has been removed from this da.

Release required?

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

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.

@kierramarie
Copy link
Author

/run pipeline

@brendankellyibm
Copy link
Contributor

@kierramarie That variable is being used here too - https://github.com/terraform-ibm-modules/terraform-ibm-rag-sample-da/blob/main/modules/watson-machine-learning/main.tf#L56

That will need to be refactored before we can remove the variable

@kierramarie
Copy link
Author

@ocofaigh what should be done here?

@ocofaigh
Copy link
Member

@kierramarie please work with the codeowner @brendankellyibm to determine next steps

@brendankellyibm
Copy link
Contributor

@kierramarie @ocofaigh can I get more of an understanding on this issue? I see that the watsonX SaaS DA has this flag now, but is that not for the default project created in that DA?

We're still creating a different WatsonX project in this DA, so does it not make sense to keep the option intact?

@ocofaigh
Copy link
Member

ocofaigh commented Nov 19, 2024

@kierramarie please include the issue link with the details / requirements of this change

@kierramarie
Copy link
Author

kierramarie commented Nov 19, 2024

Here is the issue: terraform-ibm-modules/terraform-ibm-watsonx-saas-da#167

I was asked to remove the variable from the rag sample da in that issue.

@brendankellyibm
Copy link
Contributor

@in-1911 @ocofaigh @kierramarie

I think we still need the variable itself, as we're creating a project that the user may want to keep sensitive. We can refactor the code in light of this variable - in fact it seems we're not using the configure_project module from the SaaS DA when it would be better to. Let me know if I'm wrong in any assumption here.

@in-1911
Copy link
Contributor

in-1911 commented Nov 20, 2024

I needed this settings for a project in our lab demo scenarios to pass SCC scan, if this is implemented / deployed in Watson SaaS DA now, we can remove the whole project provisioning from the Sample App Config DA.
The only concern is that from Watson SaaS DA the project name is "demo" and does not use prefix - but that would be a minor issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants