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

Remove unused async fields #17098

Open
melinath opened this issue Jan 24, 2024 · 5 comments · May be fixed by GoogleCloudPlatform/magic-modules#12517
Open

Remove unused async fields #17098

melinath opened this issue Jan 24, 2024 · 5 comments · May be fixed by GoogleCloudPlatform/magic-modules#12517
Assignees
Labels
fixit-technical-debt mmv1-generator Provider-wide changes to resource templates or other generator changes service/terraform size/s technical-debt
Milestone

Comments

@melinath
Copy link
Collaborator

We currently have a number of fields in our async block that don't have any impact on the generator. For example:

async: !ruby/object:Api::OpAsync
 operation: !ruby/object:Api::OpAsync::Operation
   base_url: '{{op_id}}'
 # The following are all unused.
   path: 'unused'
   wait_ms: 0  # unused
 result: !ruby/object:Api::OpAsync::Result
   path: 'unused'
 status: !ruby/object:Api::OpAsync::Status
   path: 'unused'
   allowed: []
 error: !ruby/object:Api::OpAsync::Error
   path: 'unused'
   message: 'unused'

These particular fields were previously required, but were made optional to resolve #14976. There may also be other unused fields.

We should remove all fields that are not used by the generator, since it causes extra work and confusion for contributors.

@melinath
Copy link
Collaborator Author

melinath commented Aug 5, 2024

#18954 was partially cause by / obfuscated by the large number of unused fields present.

@melinath
Copy link
Collaborator Author

melinath commented Oct 21, 2024

I'm pretty sure kind is also unused - missed it before because it was already optional.

@ScottSuarez
Copy link
Collaborator

ScottSuarez commented Dec 6, 2024

I wrote (in collaboration with AI) a small python script that helps with this.

Pull request here: GoogleCloudPlatform/magic-modules#12514

import os

def modify_yaml_files():
    products_dir = 'mmv1/products'
    
    # Fields to remove under each section
    remove_fields = {
        'operation': {'path', 'wait_ms'},
        'result': {'path'},
        'status': {'path', 'allowed'},
        'error': {'path', 'message'}
    }
    
    for product in os.listdir(products_dir):
        product_path = os.path.join(products_dir, product)
        if os.path.isdir(product_path):
            for root, _, files in os.walk(product_path):
                for file in files:
                    if file.endswith('.yaml'):
                        file_path = os.path.join(root, file)
                        print(f"\nProcessing: {file_path}")
                        
                        with open(file_path, 'r') as f:
                            lines = f.readlines()
                        
                        # Track state and indentation
                        in_async = False
                        current_section = None
                        async_indent = 0
                        section_indent = 0
                        lines_to_keep = []
                        
                        # Buffer for current section
                        current_section_lines = []
                        current_section_has_content = False
                        
                        for i, line in enumerate(lines, 1):
                            indent = len(line) - len(line.lstrip())
                            stripped = line.strip()
                            
                            # Check for async section
                            if stripped.startswith('async:'):
                                in_async = True
                                async_indent = indent
                                lines_to_keep.append(line)
                                continue
                            
                            # Check for sections under async
                            if in_async and any(stripped.startswith(f"{section}:") for section in remove_fields):
                                # Process previous section if exists
                                if current_section and current_section_has_content:
                                    lines_to_keep.extend(current_section_lines)
                                
                                current_section = next(section for section in remove_fields if stripped.startswith(f"{section}:"))
                                section_indent = indent
                                current_section_lines = [line]
                                current_section_has_content = False
                                continue
                            
                            # Process lines within a section
                            if current_section and indent > section_indent:
                                # Check if this is a field we want to remove
                                field = stripped.split(':')[0] if ':' in stripped else None
                                if field and field in remove_fields[current_section]:
                                    continue
                                
                                # If it's not a field to remove, keep track of content
                                if stripped and not stripped.startswith('#'):
                                    current_section_has_content = True
                                current_section_lines.append(line)
                            else:
                                # Leaving a section
                                if current_section:
                                    if current_section_has_content:
                                        lines_to_keep.extend(current_section_lines)
                                    current_section = None
                                    current_section_lines = []
                                    current_section_has_content = False
                                
                                # Check if we're leaving async section
                                if in_async and indent <= async_indent and stripped:
                                    in_async = False
                                
                                lines_to_keep.append(line)
                        
                        # Handle last section if exists
                        if current_section and current_section_has_content:
                            lines_to_keep.extend(current_section_lines)
                        
                        # Write back only if we made changes
                        if lines_to_keep != lines:
                            print(f"Writing changes to {file_path}")
                            with open(file_path, 'w') as f:
                                f.writelines(lines_to_keep)
                        else:
                            print(f"No changes needed in {file_path}")

if __name__ == "__main__":
    modify_yaml_files()

@ScottSuarez
Copy link
Collaborator

aync operation.(kind) and status can also be removed. PR out now.

@ScottSuarez
Copy link
Collaborator

I have a PR that removes these values from the serializer
GoogleCloudPlatform/magic-modules#12517

I'm going to wait until January to merge. This PR will close the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixit-technical-debt mmv1-generator Provider-wide changes to resource templates or other generator changes service/terraform size/s technical-debt
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants