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 EmbeddedAnsible ScmCredential parent class #22577

Conversation

agrare
Copy link
Member

@agrare agrare commented Jun 23, 2023

All EmbeddedAnisble credentials should derive from the EmbeddedAnsible::AutomationManager::Credential class.

This was causing issues with displaying all Embedded Ansible credentials by doing ManageIQ::Providers::EmbeddedAnsible::AutomationManager::Credential.all specifically in the UI.

Dependents:

All EmbeddedAnisble credentials should derive from the
`EmbeddedAnsible::AutomationManager::Credential` class.
@agrare agrare force-pushed the fix_embedded_ansible_scm_credential_parent_class branch from 9eaeb64 to 1595ebd Compare June 23, 2023 18:21
@miq-bot
Copy link
Member

miq-bot commented Jun 23, 2023

Checked commit agrare@1595ebd with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint
2 files checked, 0 offenses detected
Everything looks fine. 🍪

@Fryguy
Copy link
Member

Fryguy commented Jun 23, 2023

I don't love the mixin, but i see why it's needed as it's the only way to "share" the ScmCredential code with workflows while maintaining 2 independent hierarchies.

@Fryguy Fryguy merged commit 60bc7dc into ManageIQ:master Jun 23, 2023
9 checks passed
@Fryguy Fryguy self-assigned this Jun 23, 2023
@agrare agrare deleted the fix_embedded_ansible_scm_credential_parent_class branch June 23, 2023 19:15
@agrare
Copy link
Member Author

agrare commented Jun 23, 2023

I don't love the mixin

Yeah I agree which is why I avoided it initially, but if we want to share "SCM" type params while keeping two different class hierarchies I don't see another option.

@Fryguy
Copy link
Member

Fryguy commented Jun 28, 2023

@agrare A conflict occurred during the backport of this pull request to petrosian.

If this pull request is based on another pull request that has not been marked for backport, add the appropriate labels to the other pull request. Otherwise, please create a new pull request direct to the petrosian branch in order to resolve this.

Conflict details:

* Unmerged path app/models/manageiq/providers/embedded_automation_manager/scm_credential.rb

@Fryguy
Copy link
Member

Fryguy commented Jun 28, 2023

Backported to petrosian in commit 013f3d1.

commit 013f3d1c4fc1822b8d95d1dfcedfe5da0255bed1
Author: Jason Frey <fryguy9@gmail.com>
Date:   Fri Jun 23 15:12:40 2023 -0400

    Merge pull request #22577 from agrare/fix_embedded_ansible_scm_credential_parent_class
    
    Fix EmbeddedAnsible ScmCredential parent class
    
    (cherry picked from commit 60bc7dcd2647896b11f69a441e85dc269b3d5834)

Fryguy added a commit that referenced this pull request Jun 28, 2023
…tial_parent_class

Fix EmbeddedAnsible ScmCredential parent class

(cherry picked from commit 60bc7dc)
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