Add option to exclude package names from deletion

[Hsn723]

This PR contains the changes proposed in #8 but split into two PRs: one for each proposed feature. This one provides the ignore-package-names option allowing to provide a regular expression to specify package names to exclude from deletion. This should address use cases like those in #6. For instance, with package names

- 4.20
- 4.21
- 4.20.20200329
- 4.21.20200330

And the following configuration:

with: 
  ignored-version-names: "docker-base-layer|^\\d+\\.\\d+$"
  num-old-versions-to-delete: "5"

Only 4.20.20200329 and 4.21.20200330 would be deleted. As the current GraphQL API doesn't allow to query packages with a filter pattern, filtering is done after retrieving the last N packages. As a result, less than N packages may be deleted, which admittedly may be contrary to user expectation (though the same happens when requesting N older packages when there are less than N).

[mblarsen]

@Hsn723 did you look into a solution that exclude the versions from the query in the first place? If that is even possible.

Could you run into a situation where there'll never be any versions left. The case where numOldVersionsToDelete is smaller than the number of versions matching you regular expression? Would one have to "know your versions" and set this value higher to avoid that or what do you think?

Kind of related: Do you know if this action could remove all versions or will it at least leave one?

[Hsn723]

I did look into passing the filter into the query however the current GitHub GraphQL API v4 does not support any filtering at this time.

Could you run into a situation where there'll never be any versions left.

Indeed, there's the possibility of the oldest numOldVersionsToDelete versions all match the filtering pattern and therefore ending up with nothing to delete. Unfortunately I don't have an elegant solution other than "know your versions" as you mentioned. Or, having a versioning scheme that allows some predictability. Automatically counting up is also an option, however that will require several passes.

Do you know if this action could remove all versions or will it at least leave one?

Yes, even with the initial implementation this action can very well remove all versions, for instance if numOldVersionsToDelete is set higher to the total number of packages. Leaving at least one (the latest version) would indeed be desirable, although probably out of the scope of the current PR (I can gladly prepare another PR for that though).

[mblarsen]

Thanks for your valuable replies. I can see the complexities.

I find it odd that the default implementation would let you delete all versions. I'm afraid to use the action without this "protection".

Also, the numOldVersionsToDelete is kind of up-side-down isn't it? Wouldn't it make more sense with a keepNumVersions + what your PR providers?

Leaving at least one (the latest version) would indeed be desirable, although probably out of the scope of the current PR

Yeah, that would be a different PR.

[Hsn723]

keepNumVersions has also been proposed in #5, also that one is also tricky with the current GitHub GraphQL API v4 as the API doesn't provide a way to return "all" package versions unless you call the API asking for the "first N packages" with a large enough number.

By the way, if #5 gets implemented it would also be possible to combine it with the ignore-package-names proposed in this PR.

[mblarsen]

API doesn't provide a way to return "all" package versions unless you call the API asking for the "first N packages" with a large enough number.

I guess any, not necessarily large, number would suffice. As long as you don't add more than whatever that number is. At each delete action execution it would carve away old versions until there are no more of them. Isn't that how it would/could work?

By the way, if #5 gets implemented it would also be possible to combine it with the ignore-package-names proposed in this PR.

Good to know, thank you.

[Hsn723]

At each delete action execution it would carve away old versions until there are no more of them.

In theory yes, for instance if you set keepNumVersions to 5 and call the API every time requesting the 10 latest versions. What could possibly betray user expectations however is that this way you would end up deleting old (but not oldest) packages and slowly working your way down when the user would rather have oldest packages deleted first. Though the reality is that automation with the current API has no "one size fits all" and some users' expectations may be betrayed regardless.

[mblarsen]

I see. Thanks for the explanation.

[mblarsen]

@Hsn723 have you been using your branch. Seems like there is nothing happening in this repo.

UPDATE using your branch now in an action and it works great. I hope this will get merged.

[Hsn723]

@Hsn723 have you been using your branch. Seems like there is nothing happening in this repo.

UPDATE using your branch now in an action and it works great. I hope this will get merged.

Thanks for the feedback. We've been using a branch (https://github.com/MatsuriJapon/delete-package-versions/tree/matsuri) that contains this and other proposed changes (#9) in production for a while now and haven't encountered issues so far.

[cprell]

@Hsn723 Somehow it's not working for me.

  uses: myfork/delete-package-versions@matsuri
  name: delete package
  with:
    package-name: 'name'
    num-old-versions-to-delete: 1
    ignored-version-names: docker-base-layer|^[1-9]+\.[0-9]+\.[0-9]+$
    search-range: 50

This is what I am using. The action says the following:
number of versions requested was: 50, but found: 24
version with id: xxx, deleted

When I look at the packages, the version doesn't get deleted.
I am able to delete the version manually by using the package-id however.

Do you have any ideas? Thanks in advance.

[Hsn723]

@cprell You're right!
It looks like there was a bug in our dry-run implementation that made everything a dry-run, which we've addressed.

Also btw, you'll need to escape slashes inside your regular expression as it seems that otherwise the slashes get consumed once by the input parser.

I'd also like to advise against using the "matsuri" branch of our fork. We use it internally and may change it periodically (for instance rebasing with any upstream changes, which was why we cut that branch in the first place). I'm pleasantly surprised to see it being used elsewhere, but cannot assume responsibility for any broken builds 🙏.

[cprell]

@Hsn723 Thanks for the quick reply!

Also btw, you'll need to escape slashes inside your regular expression as it seems that otherwise the slashes get consumed once by the input parser.

In this case I think the regex works correctly for what I want to achieve. It filters out release versions like 1.2.0 but not snapshot versions like 1.2.0-SNAPSHOT.

I'd also like to advise against using the "matsuri" branch of our fork. We use it internally and may change it periodically

Thanks for the heads up, but I forked your fork and use our fork, so that I can use a stable version without any changes. I think using the matsuri branch is fine then. (and I like the branch name!)

I'm pleasantly surprised to see it being used elsewhere, but cannot assume responsibility for any broken builds 🙏.

Of course, you've done great work. Thanks for that!
I tried using the action now and it worked. Big help!

[NamrataJha]

This feature has been released with the latest version actions/delete-package-versions@v2. Closing this PR.