cli: add snapshot commmand line interfaces#4
Merged
jma merged 1 commit intorero:stagingfrom Jun 21, 2022
Merged
Conversation
jma
force-pushed
the
maj-es-snap
branch
4 times, most recently
from
12aa1aa to
e11df99
Compare
lauren-d
approved these changes
Jun 8, 2022
zannkukai
approved these changes
Jun 8, 2022
Contributor
zannkukai
left a comment
zannkukai
left a comment
There was a problem hiding this comment.
all my remarks are cosmetic and don't need any changes to works as expected.
| from flask.cli import with_appcontext | ||
| from invenio_search import current_search, current_search_client | ||
|
|
||
| from ...shared import abort_if_false |
Contributor
There was a problem hiding this comment.
cosmtetic : up to ".." I prefer to use absolute import
Comment on lines
+40
to
+44
| @snapshot.command('list') | ||
| @with_appcontext | ||
| @click.option('-n', '--name', help="default=_all", default='_all') | ||
| @click.argument('repository') | ||
| def list_snapshot(repository, name): |
Contributor
There was a problem hiding this comment.
cosmetic : Personnaly, I don't like to put logical code into "init.py" when it's not necessary. Could be better to put these methods into a cli.py fil, import methods and use __all__ buildin variable ?
Comment on lines
+50
to
+59
| infos = [] | ||
| for snapshot in snapshots['snapshots']: | ||
| infos.append({ | ||
| 'snapshot': snapshot['snapshot'], | ||
| 'start_time': snapshot['start_time'], | ||
| 'duration': snapshot['duration_in_millis'] / 1000, | ||
| 'shards': snapshot['shards'], | ||
| 'state': snapshot['state'], | ||
| 'uuid': snapshot['uuid'] | ||
| }) |
Contributor
There was a problem hiding this comment.
cosmetic : Sourcery plugin would suggest a list comprehension for this.
infos = [{
'snapshot': snapshot['snapshot'],
'start_time': snapshot['start_time'],
'duration': snapshot['duration_in_millis'] / 1000,
'shards': snapshot['shards'],
'state': snapshot['state'],
'uuid': snapshot['uuid']
}
for snapshot in snapshots['snapshots']
]
rerowep
approved these changes
Jun 8, 2022
BadrAly
approved these changes
Jun 9, 2022
* Uses v3 of setup-python for github action. * Adds a custom docker-services to configure a snapshot volume for tests. * Adds a shared code for all the cli commands. * Adds fixtures tests elasticsearch indexes. Co-Authored-by: Johnny Mariéthoz <Johnny.Mariethoz@rero.ch>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
tests.
Co-Authored-by: Johnny Mariéthoz Johnny.Mariethoz@rero.ch