Page MenuHomePhabricator

[native-rust-library] Backup service config
ClosedPublic

Authored by michal on Dec 8 2023, 7:33 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 12, 8:53 PM
Unknown Object (File)
Thu, Nov 7, 12:26 AM
Unknown Object (File)
Fri, Oct 18, 2:43 AM
Unknown Object (File)
Fri, Oct 18, 2:43 AM
Unknown Object (File)
Fri, Oct 18, 2:43 AM
Unknown Object (File)
Fri, Oct 18, 2:42 AM
Unknown Object (File)
Fri, Oct 18, 2:42 AM
Unknown Object (File)
Oct 12 2024, 7:41 AM
Subscribers

Details

Summary

ENG-5346

This diff introduces the config for backup URL and refactors the existing identity service config code so that it can be reused between identity and backup.

Test Plan

Added assert_eq!(BACKUP_SOCKET_ADDR, ""); in the test to get the current value of BACKUP_SOCKET_ADDR in the panic message. Then I've run these command and checked the value of BACKUP_SOCKET_ADDR:

  • cargo test --release, no file -> backup.commtechnologies.org
  • cargo test --release, file -> value in the file
  • cargo test, file -> value in the file
  • cargo test, no file -> backup.staging.commtechnologies.org

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

Also @varun, a question: other facts folders are .gitignored. Do you think it makes sense to do it for this one too?

michal requested review of this revision.Dec 8 2023, 9:03 AM

Also @varun, a question: other facts folders are .gitignored. Do you think it makes sense to do it for this one too?

yeah we should .gitignore this facts folder too, good call out

native/native_rust_library/build.rs
147–150 ↗(On Diff #34435)

should we be more specific with these panic messages?

This revision is now accepted and ready to land.Dec 11 2023, 9:13 AM
bartek added 1 blocking reviewer(s): varun.

Nice! At first glance feels like over-engineering, but it's gonna pay back in the future

native/native_rust_library/build.rs
147–150 ↗(On Diff #34435)

+1, error message will contain line number but it's better to have it explicitly say Couldn't write backup service config

This revision now requires review to proceed.Dec 12 2023, 4:47 AM

accepting again but please incorporate @bartek 's suggested panic message and .gitignore the facts folder. thanks!

This revision is now accepted and ready to land.Dec 12 2023, 8:16 AM

Worth noting that we don't .gitignore lib/facts. But assuming we're talking about native/facts here, I think it makes sense to .gitignore.

Looks like native/.gitignore has native/facts/alchemy.json covered, but it should probably just list the whole native/facts folder.

Add the whole native/facts to .gitignore, improve error messages