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

With update_existing=false, evergreen ignores existing config file if the extension doesn't match the first one defined in filename_list #208

Closed
detiber opened this issue Aug 28, 2024 · 3 comments · Fixed by #210
Assignees
Labels
bug Something isn't working

Comments

@detiber
Copy link

detiber commented Aug 28, 2024

Describe the bug

It looks like a bug was introduced post 1.10.8 with the introduction of support for updating existing configs, when update_existing is false and the existing configuration file extension does not match the first one defined in filename_list:

evergreen/evergreen.py

Lines 82 to 89 in ff2f3ff

existing_config = None
filename_list = [".github/dependabot.yaml", ".github/dependabot.yml"]
dependabot_filename_to_use = filename_list[0] # Default to the first filename
for filename in filename_list:
existing_config = check_existing_config(repo, filename, update_existing)
if existing_config:
dependabot_filename_to_use = filename
break

evergreen/evergreen.py

Lines 210 to 234 in ff2f3ff

def check_existing_config(repo, filename, update_existing):
"""
Check if the dependabot file already exists in the
repository and return the existing config if it does
Args:
repo (github3.repos.repo.Repository): The repository to check
filename (str): The dependabot configuration filename to check
update_existing (bool): Whether to update existing dependabot configuration files
Returns:
github3.repos.contents.Contents | None: The existing config if it exists, otherwise None
"""
existing_config = None
try:
existing_config = repo.file_contents(filename)
if existing_config.size > 0:
if not update_existing:
print(
"Skipping " + repo.full_name + " (dependabot file already exists)"
)
return None
except github3.exceptions.NotFoundError:
pass
return existing_config

Because check_existing_config eats the exception and returns None when the file doesn't exist and also returns None when the file exists AND update_existing=false, dependabot_filename_to_use is always filename_list[0], so if there is a pre-existing file with the name filename_list[1], then a PR will be created despite a dependabot config already being present.

This also results in the confusing job output:

Skipping org/repo (dependabot file already exists)
Checking org/repo for compatible package managers
	Created pull request https://github.com/org/repo/pull/41

To Reproduce

See above

Expected behavior

Dependabot not to create a PR when update_existing=false, a dependabot config already exists, and the existing file has any supported file extension, not just the default one.

Screenshots

No response

Additional context

I'd suggest that either:

  • check_existing_config provide the caller a way to differentiate between the file not existing and it existing, but should not be updated
  • refactor the calling code to handle testing for existing config vs whether to update the existing config, similar to below:
        for filename in filename_list:
            existing_config = check_existing_config(repo, filename)
            if existing_config:
                dependabot_filename_to_use = filename

        if existing_config and not update_existing:
            print(
                "Skipping " + repo.full_name + " (dependabot file already exists)"
            )
            continue
def check_existing_config(repo, filename, update_existing):
    """
    Check if the dependabot file already exists in the
    repository and return the existing config if it does

    Args:
        repo (github3.repos.repo.Repository): The repository to check
        filename (str): The dependabot configuration filename to check

    Returns:
        github3.repos.contents.Contents | None: The existing config if it exists, otherwise None
    """
    existing_config = None
    try:
        existing_config = repo.file_contents(filename)
        if existing_config.size > 0:
            return existing_config
        else:
            return None
    except github3.exceptions.NotFoundError:
        pass
    return existing_config

Do note, that I have not even verified the above code will parse, let alone function correctly.

@zkoppert
Copy link
Member

@detiber Thanks for opening this bug report! I'm taking a look into this today.

@zkoppert zkoppert self-assigned this Aug 30, 2024
@zkoppert
Copy link
Member

zkoppert commented Aug 30, 2024

Pull request open for review! Thanks again @detiber !

@detiber
Copy link
Author

detiber commented Aug 30, 2024

Thanks for the quick fix :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants