-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add the delete_missing_records option. #47
Conversation
for record_id in to_delete: | ||
batch.delete_record(id=record_id, | ||
bucket=bucket_id, | ||
collection=collection_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice if we could generalize it to bucket/collections/groups
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For groups it might make sense, but for buckets and collections I would rather not trust kinto-wizard for such a dangerous task 😂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have created #48 for that purpose. I believe we might need specifics option for each --delete-records
--delete-groups
--delete-collections
etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather not trust kinto-wizard for such a dangerous task joy
Why would be buckets so different? If we prompt, I don't see the problem...
if not force: | ||
message = ("Are you sure that you want to delete the " | ||
"following {} records?".format(len(list(to_delete)))) | ||
value = input(message) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't remember but we may have an option to always accept prompts, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We currently don't have any prompts. We might want to add a -y --yes
option though.
kinto_wizard/__main__.py
Outdated
@@ -29,6 +29,9 @@ def main(): | |||
subparser.add_argument('--force', | |||
help='Load the file using the CLIENT_WINS conflict resolution strategy', | |||
action='store_true') | |||
subparser.add_argument('--delete', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@leplatrem Do you want me to use something less generic here to be future proof?
--delete-missing-records
--delete-records
for record_id in to_delete: | ||
batch.delete_record(id=record_id, | ||
bucket=bucket_id, | ||
collection=collection_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather not trust kinto-wizard for such a dangerous task joy
Why would be buckets so different? If we prompt, I don't see the problem...
kinto_wizard/__main__.py
Outdated
@@ -32,6 +32,9 @@ def main(): | |||
subparser.add_argument('--dry-run', | |||
help="Do not apply write call to the server", | |||
action='store_true') | |||
subparser.add_argument('--delete', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--delete-record
yes
kinto_wizard/yaml2kinto.py
Outdated
@@ -142,6 +144,25 @@ | |||
collection=collection_id, | |||
data=record_data, | |||
permissions=record_permissions) | |||
if delete_missing_records and collection_records and \ | |||
'records' in current_collection: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit™: intermediary variable ;)
CHANGELOG.rst
Outdated
@@ -8,6 +8,8 @@ This document describes changes between each past release. | |||
|
|||
- Add a ``--dry-run`` for the load command to see how many records | |||
would be deleted. (#46) | |||
- Add a ``--delete-record`` to remove existing records in the | |||
collections we are importing. (#47) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something like To delete the existing records that are not in the YAML file would be better maybe ?
kinto_wizard/yaml2kinto.py
Outdated
message = ("Are you sure that you want to delete the " | ||
"following {} records?".format(len(list(to_delete)))) | ||
value = input(message) | ||
print(to_delete) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want this print?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I figured it would give extra information about the records that are going to be deleted. But I could do it only in verbose mode maybe?
Fixes #44