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.

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

Migration for node hash invalidation #2136

Merged
merged 7 commits into from
Nov 5, 2018
Merged

Conversation

szoupanos
Copy link
Contributor

This is a fix for issue #2131

@coveralls
Copy link

coveralls commented Oct 26, 2018

Coverage Status

Coverage decreased (-0.03%) to 68.686% when pulling 8ce0865 on szoupanos:issue_2131 into 1337bc2 on aiidateam:develop.

@giovannipizzi
Copy link
Member

There are a number of failing tests

@sphuber sphuber self-requested a review October 30, 2018 10:37
@sphuber
Copy link
Contributor

sphuber commented Oct 30, 2018

We are going to put this on hold until #2124 has been addressed, because we run the risk that if that really poses a problem, we will have to issue another migration after having merged this first

operations = [
migrations.RunPython(notify_user, reverse_code=notify_user),
migrations.RunSQL(
""" DELETE FROM db_dbextra WHERE key='""" + _HASH_EXTRA_KEY + """';""",
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is better to hard code the hash key. If at some point we change the hash key this migration will not work. Even though at that point we will have to add another migration, which would render this one obsolete. Still there would be a scenario possible where the change in key is done before the migration and then this migration is executed, thereby missing all the current hashes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good point. Thanks



def notify_user(apps, schema_editor):
echo_warning("Invalidating all the hashes of all the nodes. Please " "run verdi rehash", bold=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Obsolete double quotes (twice) in single string

conn = op.get_bind()

# Invalidate all the hashes & inform the user
echo_warning("Invalidating all the hashes of all the nodes. " "Please run verdi rehash", bold=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

unnecessary double quotes

@sphuber sphuber merged commit 304876e into aiidateam:develop Nov 5, 2018
@szoupanos szoupanos deleted the issue_2131 branch February 9, 2019 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants