[identity] Use DB conversion helpers from comm-lib
ClosedPublic
Actions

Authored by bartek on Dec 22 2023, 5:51 AM.

Details

Reviewers

varun
michal

Commits

rCOMM64ee95ff49d3: [identity] Use DB conversion helpers from comm-lib

Summary

This finally resolves ENG-5476.

Used database conversion traits from comm-lib wherever possible. For some places, I introduced the deprecated attribute. This diff could grow too large.

Depends on D10447

Test Plan

Identity compiles and runs. Commtest passes, also played with it manually.

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

bartek created this revision.Dec 22 2023, 5:51 AM

bartek held this revision as a draft.

Herald added subscribers: tomek, ashoat. · View Herald TranscriptDec 22 2023, 5:51 AM

In the future we might consider refactoring parse_{some_business_logic_name}_attribute() too, but it's not a direct purpose of this task, so I replaced only these "universal" functions

services/identity/src/database.rs
1697	clippy fix
services/identity/src/database/device_list.rs
380–393	Refactoring this one blew my mind 🤯
services/identity/src/error.rs
33–34	This one is mainly used in code that is going to be removed anyway as a part of ENG-5842 so for now I deprecated it.

bartek added inline comments.Dec 22 2023, 6:27 AM

services/identity/src/database.rs
1352–1359	Ahh I realized I can do even better
services/identity/src/database/device_list.rs
38–42	This enum turns out to be unused

Harbormaster failed remote builds in B25322: Diff 34976!Dec 22 2023, 6:47 AM

In the future we might consider refactoring parse_{some_business_logic_name}_attribute() too, but it's not a direct purpose of this task, so I replaced only these "universal" functions

Can we make a task for it? (or possibly even just do it in another diff, seems like it would be quick)

services/identity/src/database.rs
1148–1151	Nit: does this work?
1574	Why are we removing `parse_string_attribute` but not this one?

This revision is now accepted and ready to land.Jan 2 2024, 4:21 AM

bartek added a comment.Jan 5 2024, 1:47 AM

This comment was removed by bartek.

services/identity/src/database.rs
1148–1151	Yep, missed this, thanks
1574	Still used in yet-to-be-removed code

In D10448#303431, @michal wrote:

In the future we might consider refactoring parse_{some_business_logic_name}_attribute() too, but it's not a direct purpose of this task, so I replaced only these "universal" functions

Can we make a task for it? (or possibly even just do it in another diff, seems like it would be quick)

Yeah I'll do that

Apply suggestion: simplify one more place

Harbormaster completed remote builds in B25534: Diff 35281.Jan 5 2024, 2:14 AM

Closed by commit rCOMM64ee95ff49d3: [identity] Use DB conversion helpers from comm-lib (authored by bartek). · Explain WhyJan 5 2024, 2:26 AM

This revision was automatically updated to reflect the committed changes.

bartek added a commit: rCOMM64ee95ff49d3: [identity] Use DB conversion helpers from comm-lib.