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

Shorten label on some loops to show name instead of entire json block #1002

Merged
merged 8 commits into from
Dec 16, 2024

Conversation

MallocArray
Copy link
Contributor

The current label of the step to wait for the async job templates to complete shows the full JSON output of the job var, which is very verbose and can be especially long when a survey is included.

Shorten the output so it only shows the name of the job template, very similar to the task that manages controller job templates

What does this PR do?

The current label of the step to wait for the async job templates to complete shows the full JSON output of the job var, which is very verbose and can be especially long when a survey is included.

Shorten the output so it only shows the name of the job template, very similar to the task that manages controller job templates

Sample of current very verbose output from a single job template

TASK [infra.aap_configuration.controller_job_templates : Managing Job Templates | Wait for finish the Job Templates management] ******************************************
ok: [awx-scratch4.domain.com] => (item=Create/Update Job Template {'failed': 0, 'started': 1, 'finished': 0, 'ansible_job_id': 'j689290788343.47537', 'results_file': '/root/.ansible_async/j689290788343.47537', 'changed': False, '__controller_template_item': {'name': 'Active Directory - Create CIFS Share AD Access Groups', 'description': 'Creates a computer access groups for the specified NAS Server and Share in Active Directory.', 'job_type': 'run', 'inventory': 'ActiveDirectory', 'project': 'Ansible - Development', 'playbook': 'ActiveDirectory/create_cifs_shares_access_group.yml', 'credentials': ['App Registration'], 'notification_templates_error': ['StorageTeam Notification'], 'labels': ['activedirectory', 'netapp'], 'survey_enabled': False, 'ask_variables_on_launch': True, 'roles': [{'team': 'Storage Team', 'role': 'execute'}]}, 'ansible_loop_var': '__controller_template_item'} | Wait for finish the Job Template creation)

After this change to only show the Job Template name

TASK [infra.aap_configuration.controller_job_templates : Managing Job Templates | Wait for finish the Job Templates management] ******************************************
ok: [awx-scratch4.smrcy.com] => (item=Create/Update Job Template Active Directory - Create CIFS Share AD Access Groups | Wait for finish the Job Template creation)

How should this be tested?

Is there a relevant Issue open for this?

resolves #[number]

Other Relevant info, PRs, etc

@MallocArray MallocArray requested a review from a team as a code owner December 9, 2024 22:38
@MallocArray
Copy link
Contributor Author

There are other roles that are also showing the full JSON output as part of the label. If this shortened version is acceptable, I'm willing to open future PRs to similarly update them.

Copy link
Collaborator

@Tompage1994 Tompage1994 left a comment

Choose a reason for hiding this comment

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

This looks good to me. However before accepting it would be good to apply the same change to all the roles if you're happy to do so? I'd like to see that to ensure there is consistency throughout

Thanks for the contribution

Copy link
Collaborator

@djdanielsson djdanielsson left a comment

Choose a reason for hiding this comment

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

Also add a changelog fragment with a summary of the change

@MallocArray
Copy link
Contributor Author

I'm still moving into this collection and only doing the controller section. I'm happy to update the controller roles as I run across them, and while I'll be using most of them, I'm not necessarily using all that may need updated.

I'll continue working through them and add a changelog fragment

The current label of the step to wait for the async job templates to complete shows the full JSON output of the job var, which is very verbose and can be especially long when a survey is included.

Shorten the output so it only shows the name of the job template, very similar to the task that  manages controller job templates
@MallocArray
Copy link
Contributor Author

Updated all instances that I found in my testing.
I haven't done a changelog fragment like this before, so I gave it a shot.

@MallocArray MallocArray changed the title Shorten label of job template async status Shorten label on some loops to show name instead of entire json block Dec 13, 2024
Copy link
Collaborator

@djdanielsson djdanielsson left a comment

Choose a reason for hiding this comment

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

LGTM

@sean-m-sullivan
Copy link
Collaborator

Changelog fragments can be a list of each one changed, if that makes things easier, let us know when you want to merge this, thanks for the work!

Copy link
Contributor Author

@MallocArray MallocArray left a comment

Choose a reason for hiding this comment

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

Addressed other tasks to improve loop label

@MallocArray
Copy link
Contributor Author

Are the fragments ok as they are now with separate files reflecting which role they relate to, or do you want only a single file with a list of the change on each role?

I'm not sure how to mark Tompage1994's requested changes as complete

@djdanielsson
Copy link
Collaborator

The fragments could be one file or kept as is, I think his point was just an FYI so you know that you don't have to create multiple files. @Tompage1994 needs to approve that you made the requested changes, there is nothing you can do on your side.

@Tompage1994 Tompage1994 merged commit 0d0c5df into redhat-cop:devel Dec 16, 2024
9 checks passed
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