Skip to content
This repository has been archived by the owner on Jul 13, 2023. It is now read-only.

feat: port migrate user to Rust, remove Python calling #1237

Merged
merged 2 commits into from
May 17, 2018

Conversation

bbangert
Copy link
Member

Closes #1206

@bbangert bbangert requested review from pjenvey and jrconlin May 15, 2018 23:24
let cur_month = current_message_month.to_string();
let cur_month1 = cur_month.clone();
let cur_month2 = cur_month.clone();
let cur_month3 = cur_month.clone();
Copy link
Member Author

Choose a reason for hiding this comment

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

Yes I hate all these clones, the joy of static closures.

@codecov-io
Copy link

codecov-io commented May 15, 2018

Codecov Report

Merging #1237 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #1237    +/-   ##
=======================================
  Coverage     100%    100%            
=======================================
  Files          60      60            
  Lines       10211   10328   +117     
=======================================
+ Hits        10211   10328   +117
Impacted Files Coverage Δ
autopush/tests/test_rs_integration.py 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 579ebc4...9ae1fb5. Read the comment docs.

let response = data.srv.ddb.migrate_user(
&webpush.uaid,
&webpush.message_month,
&data.srv.opts.current_message_month,
Copy link
Member

Choose a reason for hiding this comment

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

so opts.current_message_month is essentially hardcoded at startup in autopush_rs.

ddb.migrate_user doesn't need to return it, then. await_migrate_user can assign webpush.message_month = data.srv.opts.current_message_month.clone(); instead

Copy link
Member Author

Choose a reason for hiding this comment

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

That's true.

@@ -223,6 +223,10 @@ pub extern "C" fn autopush_server_new(
.expect("poll interval cannot be 0"),
};
opts.message_table_names.sort_unstable();
opts.current_message_month = opts.message_table_names
Copy link
Member

Choose a reason for hiding this comment

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

a number of the db methods do this same call to get current_message_month. we can improve that later though since they're getting refactored

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, and they do weird things since its not 'for-sure' that there's a last() on the vec, while now its guaranteed there'll be a current month.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants