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.

Sign up for GitHub

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

Delete yum artifacts regex #72

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open

Delete yum artifacts regex #72

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
6ac9c06
Add repository del_assets command
Nov 8, 2019
0cca23d
Add variant to use wildcard
Nov 8, 2019
a67c777
Improve error handling from Groovy script; use f-strings to address r…
Nov 8, 2019
6f19be9
Merge new delete command with previous one
Nov 23, 2019
26e45ce
Merge branch 'master' into delete_yum_artifacts_regex
Nov 23, 2019
6569a9c
fix whitespaces
Nov 23, 2019
d8913b6
Fix some obvious problems
f18m Nov 24, 2019
cb9d3a5
Merge branch 'master' into delete_yum_artifacts_regex
Dec 14, 2019
4087402
Restore functionalities after integrating fixes from the MASTER branch.
f18m Dec 14, 2019
b987072
Remove previous delete command implementation (not going through APIs
f18m Dec 14, 2019
d7e6ebf
Fix linter warnings
f18m Dec 14, 2019
806cfa0
Fix for tests
f18m Dec 15, 2019
acf6586
Address reviewers comments
f18m Dec 29, 2019
32e59be
Fix integration tests
f18m Dec 29, 2019
0ccbed4
Fix unit tests
f18m Dec 30, 2019
2a70340
Fix flake8 errors/warnings; improve integration test to verify that t…
f18m Dec 30, 2019
1c9fabe
Merge branch 'master' into delete_yum_artifacts_regex
f18m Dec 30, 2019
0b1d9da
Fix more flake8 errors
f18m Dec 30, 2019
b9fe5cc
Fix CodeFactor error
f18m Dec 30, 2019
0506853
more flake8 fixes
f18m Dec 30, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions src/nexuscli/api/repository/collection.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

SCRIPT_NAME_CREATE = 'nexus3-cli-repository-create'
SCRIPT_NAME_DELETE = 'nexus3-cli-repository-delete'
SCRIPT_NAME_DELETE_ASSETS = 'nexus3-cli-repository-delete-assets'


def get_repository_class(raw_repo):
Expand Down Expand Up @@ -146,6 +147,41 @@ def delete(self, name):
self._client.scripts.create_if_missing(SCRIPT_NAME_DELETE, content)
self._client.scripts.run(SCRIPT_NAME_DELETE, data=name)

def delete_assets(self, reponame, assetRegex, isWildcard, dryRun):
"""
Delete assets from a repository through a Groovy script

:param reponame: name of the repository to delete assets from.
:type reponame: str
:param assetRegex: wildcard for assets to delete
:type assetRegex: str
:param isWildcard: is the assetRegex a regex or a wildcard?
:type isWildcard: bool
:param dryRun: do a dry run or delete for real?
:type dryRun: bool
"""
content = nexus_util.groovy_script(SCRIPT_NAME_DELETE_ASSETS)
self._client.scripts.delete(SCRIPT_NAME_DELETE_ASSETS)
self._client.scripts.create_if_missing(SCRIPT_NAME_DELETE_ASSETS, content)

# prepare JSON for Groovy:
jsonData = {}
f18m marked this conversation as resolved.
Show resolved Hide resolved
jsonData['repoName']=reponame
jsonData['assetRegex']=assetRegex
jsonData['isWildcard']=isWildcard
jsonData['dryRun']=dryRun
groovy_returned_json = self._client.scripts.run(SCRIPT_NAME_DELETE_ASSETS, data=json.dumps(jsonData))

# parse the JSON we got back
if 'result' not in groovy_returned_json:
raise exception.NexusClientAPIError(groovy_returned_json)
script_result = json.loads(groovy_returned_json['result']) # this is actually a JSON: convert to Python dict
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe there's a .json method in the requests' response that would be preferable to calling json.loads directly.

Copy link
Author

Choose a reason for hiding this comment

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

I think that ScriptCollection.run() is already doing .json() on the HTTP response from the Nexus. However I think that the response.result field is interpreted as a string and does not handle the case whether that string is another JSON... hence the json.loads()...

if 'assets' not in script_result:
raise exception.NexusClientAPIError(groovy_returned_json)

assets_list = script_result['assets']
return assets_list

def create(self, repository):
"""
Creates a Nexus repository with the given format and type.
Expand Down
83 changes: 83 additions & 0 deletions src/nexuscli/api/script/groovy/nexus3-cli-repository-delete-assets.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
// Original from:
// https://github.com/hlavki/nexus-scripts
// Modified to include some improvements to logging, option to do a "dry run",etc

import org.sonatype.nexus.repository.storage.Asset
import org.sonatype.nexus.repository.storage.Query
import org.sonatype.nexus.repository.storage.StorageFacet
import org.sonatype.nexus.repository.raw.internal.RawFormat

import groovy.json.JsonOutput
import groovy.json.JsonSlurper

def log_prefix = "nexus3-cli GROOVY SCRIPT: "

// https://gist.github.com/kellyrob99/2d1483828c5de0e41732327ded3ab224
// https://gist.github.com/emexelem/bcf6b504d81ea9019ad4ab2369006e66

def request = new JsonSlurper().parseText(args)
assert request.repoName: 'repoName parameter is required'
assert request.assetRegex: 'name regular expression parameter is required, format: regexp'
assert request.isWildcard != null: 'isWildcard parameter is required'
assert request.dryRun != null: 'dryRun parameter is required'

def repo = repository.repositoryManager.get(request.repoName)
if (repo == null) {
log.warn(log_prefix + "Repository ${request.repoName} does not exist")
return
}
//assert repo.format instanceof RawFormat: "Repository ${request.repoName} is not raw, but ${repo.format}"
log.info(log_prefix + "Valid repository: ${request.repoName}")

StorageFacet storageFacet = repo.facet(StorageFacet)
def tx = storageFacet.txSupplier().get()

try {
tx.begin()

log.info(log_prefix + "Gathering list of assets from repository: ${request.repoName} matching pattern: ${request.assetRegex} isWildcard: ${request.isWildcard}")
Iterable<Asset> assets
if (request.isWildcard)
assets = tx.findAssets(Query.builder().where('name like ').param(request.assetRegex).build(), [repo])
else
assets = tx.findAssets(Query.builder().where('name MATCHES ').param(request.assetRegex).build(), [repo])

def urls = assets.collect { "/repository/${repo.name}/${it.name()}" }

if (request.dryRun == false) {
// add in the transaction a delete command for each asset
assets.each { asset ->
log.info(log_prefix + "Deleting asset ${asset.name()}")
tx.deleteAsset(asset);

def assetId = asset.componentId()
if (assetId != null) {
def component = tx.findComponent(assetId);
if (component != null) {
log.info(log_prefix + "Deleting component with ID ${assetId} that belongs to asset ${asset.name()}")
tx.deleteComponent(component);
}
}
}
}

tx.commit()
log.info(log_prefix + "Transaction committed successfully")

def result = JsonOutput.toJson([
assets : urls,
assetRegex : request.assetRegex,
repoName : request.repoName
])
return result

} catch (all) {
log.warn(log_prefix + "Exception: ${all}")
all.printStackTrace()
log.info(log_prefix + "Rolling back changes...")
tx.rollback()
log.info(log_prefix + "Rollback done.")
} finally {
// @todo Fix me! Danger Will Robinson!
tx.close()
}
55 changes: 51 additions & 4 deletions src/nexuscli/cli/subcommand_repository.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
Usage:
nexus3 repository --help
nexus3 repository list
nexus3 repository (delete|del) <repo_name> [--force]
nexus3 repository create hosted (bower|npm|nuget|pypi|raw|rubygems)
<repo_name>
[--blob=<store_name>] [--strict-content] [--cleanup=<c_policy>]
Expand All @@ -24,6 +23,9 @@
[--blob=<store_name>] [--strict-content] [--cleanup=<c_policy>]
[--write=<w_policy>]
[--depth=<repo_depth>]
nexus3 repository (delete|del) <repo_name> [--force]
nexus3 repository del_assets_regex <repo_name> <assets_regex> [--force]
Copy link
Owner

Choose a reason for hiding this comment

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

Wouldn't these del_assets* subcommands be better under the existing root command (delete|del)?

  nexus3 (delete|del) <repository_path>

You could add flags for the user to specify whether the <repository_path> contains a regex or a wildcard (glob?)

I'll have a better look at the problem this weekend so I have a better informed suggestion.

Copy link
Author

@f18m f18m Nov 8, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Regarding the 2 points:

  1. subcommand vs root command: honestly as user of nexus3-cli I found very confusing having both subcommands AND root commands: I didn't pay much attention initially at the repository/script/etc subcommands documented at the end of the "--help" section and it took me a while to discover how many possibilities nexus3-cli provides beside the root commands... all in all they look at bit "hidden".
    So IMHO I would completely remove root commands and leave only subcommands: all the list/upload/download/delete root commands IMHO should be under an "assets" subcommand.
    Then in this scenario these del_assets_* commands added by this PR could be merged with the "nexus3 assets delete" command perhaps.

  2. regarding having a single command instead of del_assets_regex/del_assets_wildcard: I think you're right, having a flag would make it possible to unify them.
    Should the default be to interpret the parameter appearing after the <repo_name> as a wildcard or as a regex? Since wildcards are easier to use I would opt for the former.

Copy link
Owner

Choose a reason for hiding this comment

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

  1. All the commands used to be at the root level but I found that confusing as well. The help page was very long and most users want to upload/download/delete. So I decided to pack them in sub commands as this is already an established pattern in many cli utilities (git, for example).

As changing the CLI would be a breaking change, I’d favour a bit more research/discussion before acting on it. Perhaps build the docopt in a separate branch so we can see how it looks.

  1. I agree wildcard makes more sense. There’s an existing behaviour where it deletes everything under a matching name if the name ends with a / (eg del directory/) - if possible, I’d like to keep this behaviour.

Thanks again for your contribution.

Copy link
Author

Choose a reason for hiding this comment

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

  1. Ok sure - I understand that grouping root commands all together would be a breaking change... better not to mix with this MR. So, for now, should I keep the del_assets command under the "repository" subcommand? Or do you prefer to have a new subcommand or a root-level command for that?

  2. I have to say that after some more attempts to use the new "del_assets_wildcard" command I discovered that the OrientDB used by Nexus3 is handling wildcards in a weird (at least to me) way: i.e. the wildcard symbol % matches only string prefixes or postfixes. E.g. if I have an asset named "folder1/myasset-1.2.3.rpm" and I provide as wildcard "folder1/%1.2.3%" the Groovy script will report 0 matches. If I provide "%1.2.3%", the asset will be matched.
    All this to say: maybe allowing the user to use wildcards instead of regex is not that useful finally...

Copy link
Owner

Choose a reason for hiding this comment

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

  1. Yes please. I'd prefer to keep it consistent with the subcommand pattern. I'll take your feedback onboard and think of a way to better expose the subcommands to first-time users.

  2. I think that's still useful - if we can clearly communicate how the wildcard handling works (or link to a place that does it). One thing that I didn't consider is what happens when a file you want to delete has a wildcard character in it. I guess the user would need to escape it by default, unless we only enable these options through flags; e.g.:

  nexus3 (delete|del) <repository_path> [--regex|--wildcard] 

Options:
  --regex          Enable regular expression parsing on <repository_path>
  --wildcard       Enable wildcard parsing on <repository_path>

Copy link
Author

@f18m f18m Nov 19, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Hi @thiagofigueiro ,
just to be sure I understand: from reading the example docs you put in point #2 I assume that the answer w.r.t. point #1 is "having these del_assets_wildcard and del_assets_regex merged with the root-level 'del' command" right?

Copy link
Author

Choose a reason for hiding this comment

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

Hi @thiagofigueiro ,
I was going to implement the change but I realized now that regarding point 1

  1. if you want to have the "del_assets_wildcard/regex" commands merged with the "delete|del" command there's a small detail to fix: former cmds take the repository name and, separated, the repository path. The latter cmd is taking just the repository path. So how are user supposed to delete assets by regex?
    Would that be something like:
    nexus3 del --regex myrepoName/^[a-z]*myregex.*
    ?
    Or can we modify the "delete|del" command to separate the reponame from the asset name:
    nexus3 del --regex myrepoName ^[a-z]*myregex.*
    ?

Copy link
Owner

@thiagofigueiro thiagofigueiro Nov 22, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

--regex and --wildcard would be toggle flags to the del command.

There's existing code that can receive this

nexus3 del --regex myrepoName/^[a-z]*myregex.*

and split it using something like this.

It's a part of the code that is older and somewhat messy but it does the job. Assuming this is the docopt line:

  nexus3 (delete|del) <repository_path>

Then you could do something like this:

if options.get('--regex') or options.get('--wildcard'): 
    repository, regex_or_wildcard = self._pop_repository(options.get('<repository_path>'))
    # delete wildcard or regex code
else:
    # existing delete code

Copy link
Author

@f18m f18m Nov 23, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Hi @thiagofigueiro ,
I'm trying to do the change but I admit I'm facing several issues as I don't know the code so well.
For example: the "nexus3 (delete|del) <repository_path>" command is not using the "api/repository/*" code, while my PR has added all the code to perform delete operations in the API first.
Should I change the "delete|del" command to go through the API?
In practice such existing "delete|del" command can be implemented as a special case of the wildcard code where simply no wildcards are provided...

PS: I pushed a preliminary (untested) version where I tried to merge my delete command with previous one... if you can have a look and tell me what you think before I do the testing that would be appreciated, thanks!

nexus3 repository del_assets_wildcard <repo_name> <assets_wildcard> [--force]

Options:
-h --help This screen
Expand All @@ -37,12 +39,16 @@
-f --force Do not ask for confirmation before deleting

Commands:
repository create Create a repository using the format and options provided
repository list List all repositories available on the server
repository delete Delete a repository.
repository create Create a repository using the format and options provided
repository list List all repositories available on the server
repository delete Delete an entire repository (use with care!).
repository del_assets_regex Delete assets matching the provided regex from a repository
repository del_assets_wildcard Delete assets matching the provided wildcard (using % as wildcard) from a repository
"""
from docopt import docopt
from texttable import Texttable
import json
import sys

from nexuscli.api import repository
from nexuscli.cli import errors, util
Expand Down Expand Up @@ -129,6 +135,47 @@ def cmd_delete(nexus_client, args):
return errors.CliReturnCode.SUCCESS.value


def _cmd_del_assets(nexus_client, repoName, assetRegex, isWildcard, doForce):
"""Performs ``nexus3 repository delete_assets``"""

if not doForce:
sys.stdout.write('Retrieving assets matching {}\n'.format(assetRegex))
assets_list = nexus_client.repositories.delete_assets(repoName, assetRegex, isWildcard, True)
if len(assets_list) == 0:
sys.stdout.write('Found 0 matching assets: aborting delete\n')
return errors.CliReturnCode.SUCCESS.value

sys.stdout.write('Found {} matching assets:\n{}\n'.format(len(assets_list), '\n'.join(assets_list)))
f18m marked this conversation as resolved.
Show resolved Hide resolved
util.input_with_default(
'Press ENTER to confirm deletion', 'ctrl+c to cancel')

assets_list = nexus_client.repositories.delete_assets(repoName, assetRegex, isWildcard, False)
if len(assets_list) == 0:
sys.stdout.write('Found 0 matching assets: aborting delete\n')
return errors.CliReturnCode.SUCCESS.value

sys.stdout.write('Deleted {} matching assets:\n{}\n'.format(len(assets_list), '\n'.join(assets_list)))
return errors.CliReturnCode.SUCCESS.value


def cmd_del_assets_regex(nexus_client, args):
"""Performs ``nexus3 repository delete_assets``"""

repoName = args.get('<repo_name>')
assetRegex = args.get('<assets_regex>')
doForce = args.get('--force')
return _cmd_del_assets(nexus_client, repoName, assetRegex, False, doForce)


def cmd_del_assets_wildcard(nexus_client, args):
"""Performs ``nexus3 repository delete_assets``"""

repoName = args.get('<repo_name>')
assetWildcard = args.get('<assets_wildcard>')
doForce = args.get('--force')
return _cmd_del_assets(nexus_client, repoName, assetWildcard, True, doForce)


def main(argv=None):
"""Entrypoint for ``nexus3 repository`` subcommand."""
arguments = docopt(__doc__, argv=argv)
Expand Down