Skip to content

Conversation

@jsilhan
Copy link
Contributor

@jsilhan jsilhan commented Oct 5, 2022

No description provided.

@jsilhan
Copy link
Contributor Author

jsilhan commented Oct 5, 2022

@matllubos @zamazaljiri pls review

email_template_list = list(email_template_qs)
if self.template_slugs:
assert len(email_template_list) == len(self.template_slugs), \
"The count of email templates given does not match the count of templates found"
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like it. Which templates was not found? It can be hard to find the right one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't remember the exact template name. I created a migration and unintentionally made a typo in a template name:

operations = [
        migrations.RunPython(SyncEmailTemplates((
            'typo-in-template-name',
        ))),
    ]

Then I couldn't figure out why the template is not syncing after "successful" migration run. If there was this assert then it would be much easier to find a problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you think there's a valid usage of SyncEmailTemplates when len(email_template_list) == len(self.template_slugs) is not True?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes but you should return which slug was not found...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok. I have changed that:

...
File "/usr/local/lib/python3.9/site-packages/django/db/migrations/operations/special.py", line 190, in database_forwards
    self.code(from_state.apps, schema_editor)
  File "/usr/local/lib/python3.9/site-packages/pymess/utils/migrations.py", line 29, in __call__
    assert not email_templates_not_found, f"Email templates {email_templates_not_found} were not found."
AssertionError: Email templates {'non-existent'} were not found.

@jsilhan jsilhan force-pushed the sync-email-check-count branch 2 times, most recently from ea4c41a to e89d3a1 Compare January 4, 2023 21:36
@jsilhan jsilhan force-pushed the sync-email-check-count branch from e89d3a1 to 76b8b9e Compare January 4, 2023 22:22
@matllubos matllubos merged commit 6201b2d into druids:master Jan 25, 2023
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.

2 participants