Adtisdal/cumulus 4427 add pdr cleanup

[adtisdal-ASDC]

Summary: Summary of changes

Addresses CUMULUS-4471: Coreify PDR Cleanup task

Changes

• Add and adjusts ASDC's pdr-cleanup task to fit into cumulus core

PR Checklist

• Update CHANGELOG
• Unit tests
• Ad-hoc testing - Deploy changes and test manually
• Integration tests

---

📝 Note:
For most pull requests, please Squash and merge to maintain a clean and readable commit history.

[reweeden]

Looks pretty good! Just some ideas / food for thought mostly about best practice things that we might want to standardize...

[reweeden]

I ran into having to make the same modifications to this script. @paulpilone and I talked about possibly moving it to the top level bin folder so it doesn't need to be copy/pasted for every task. The thing I'm concerned with is slight drift between all the different versions over time.

[reweeden]

I believe our python version is 3.12. Are we trying to keep all the tasks on the same version?

[adtisdal-ASDC]

I downgraded to 3.12 to keep things consistent

[reweeden]

Should we be calling this task.py? I think in the best practices example there is a task.py and it makes sense to me for the 'entrypoint' (lambda handler) to always be in a file with the same name for all the tasks, but I'm not sure if that was actually the intention, so just an idea for discussion here.

[adtisdal-ASDC]

I wasn't sure if task.py was a filler name or not

[reweeden]

I'm also wondering about the docstring convention here. I think I ended up using the :param event: blah... convention (which I'm forgetting what it's called). I think it would be nice to pick one and have it be consistent across our python code.

[adtisdal-ASDC]

I don't have an opinion on the docstring stuff. Which ever one we pick I'll switch to!

[adtisdal-ASDC]

@bhazuka Do you have a preference?

[bhazuka]

I don't have a preference. Honestly, I generally just look for a quick summary of what the class or function is for and rely on typing for input/output unless there is something not readily intuitive.

I would say stay to a standard if you want to add params, etc. Either go with the google style guide for comments/docstings, numpy styling, or reStructuredText Docstring that looks like this

def add_numbers(x: int, y: int) -> int:
    """Returns the sum of two integers.

    :param x: The first integer.
    :type x: int
    :param y: The second integer.
    :type y: int
    :returns: The sum of the two integers.
    :rtype: int
    :raises ValueError: if x or y is not an integer.
    """
    if not isinstance(x, int) or not isinstance(y, int):
        raise ValueError("x and y must be integers")
    return x + y

Whatever you guys want to choose, let me know and I will update the best practices.

[adtisdal-ASDC]

I switched it over to the reStructuredText so they should be consistent now

[reweeden]

How do we feel about using format strings in logger calls? I thought usually the linter complains about this, but I guess it's not in the default lints or the lints we turned on.

[adtisdal-ASDC]

I'm open to switching to an alternative

[reweeden]

I've been using the '%' style formatting built in to the logging lib https://docs.astral.sh/ruff/rules/logging-f-string/

[reweeden]

We could consider turning on some additional relevant lints

[adtisdal-ASDC]

I switched formats and I added in the check here

[adtisdal-ASDC]

actually nvm I removed the lint (kept the format changes) since there's a few other places outside this PR we'd have to update the logging statements