-
Notifications
You must be signed in to change notification settings - Fork 428
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
Remove domain/retries #3774
Remove domain/retries #3774
Conversation
NelsonVides
commented
Sep 23, 2022
This comment was marked as outdated.
This comment was marked as outdated.
Codecov ReportBase: 82.82% // Head: 82.81% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #3774 +/- ##
==========================================
- Coverage 82.82% 82.81% -0.01%
==========================================
Files 531 531
Lines 34083 34095 +12
==========================================
+ Hits 28228 28236 +8
- Misses 5855 5859 +4
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
ac9aa8f
to
0e92859
Compare
This comment was marked as outdated.
This comment was marked as outdated.
0e92859
to
7193bba
Compare
This comment was marked as outdated.
This comment was marked as outdated.
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 think it's going in the right direction, but I added some comments regarding GraphQL. Also, I think we shouldn't allow re-enabling of the domains that are in deletion. I think the current code does not prevent this, and there is no test for it (I would like to have one).
Current logic for domain deletion first checks that the domain exists, then removes the domain, and then runs the remove_domain hook. But this is a problem, because if any of the hooks fails, then retrying the domain removal won’t work, because it will be stopped at the first check for domain existence. Here we instead mark the domain as "in progress for deletion", which will already disable it, and only finally delete it _after_ all hook handlers have run hopefully successfully.
7193bba
to
e365b77
Compare
This comment was marked as outdated.
This comment was marked as outdated.
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.
Looks good 👍
e365b77
to
471d300
Compare
small_tests_24 / small_tests / 471d300 small_tests_25 / small_tests / 471d300 ldap_mnesia_24 / ldap_mnesia / 471d300 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 471d300 dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 471d300 dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / 471d300 dynamic_domains_mysql_redis_25 / mysql_redis / 471d300 ldap_mnesia_25 / ldap_mnesia / 471d300 pgsql_mnesia_24 / pgsql_mnesia / 471d300 elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / 471d300 mysql_redis_25 / mysql_redis / 471d300 pgsql_mnesia_25 / pgsql_mnesia / 471d300 riak_mnesia_24 / riak_mnesia / 471d300 mssql_mnesia_25 / odbc_mssql_mnesia / 471d300 push_integration_SUITE:pubsub_ful:pm_notifications_with_inbox:inbox_msg_unread_count_apns{error,
{{assertMatch,
[{module,push_integration_SUITE},
{line,662},
{expression,"Data"},
{pattern,"# { << \"message-count\" >> := ExpectedCount }"},
{value,
#{<<"last-message-body">> => <<"Private message">>,
<<"last-message-sender">> =>
<<"alice_inbox_msg_unread_count_apns_2722@localhost">>,
<<"message-count">> => 1}}]},
[{push_integration_SUITE,check_notification,2,
[{file,
"/home/circleci/project/big_tests/tests/push_integration_SUITE.erl"},
{line,662}]},
{push_integration_SUITE,'-inbox_msg_unread_count/3-fun-0-',6,
[{file,
"/home/circleci/project/big_tests/tests/push_integration_SUITE.erl"},
{line,578}]},
{escalus_story,story,4,
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,72}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1782}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1291}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1223}]}]}} mssql_mnesia_25 / odbc_mssql_mnesia / 471d300 |