Page MenuHomePhabricator

[keyserver] Use importJSON to import olm_config.json
ClosedPublic

Authored by ashoat on May 12 2022, 6:47 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 8, 12:48 PM
Unknown Object (File)
Sat, Nov 2, 8:29 AM
Unknown Object (File)
Mon, Oct 28, 2:49 AM
Unknown Object (File)
Oct 5 2024, 1:42 AM
Unknown Object (File)
Oct 5 2024, 1:42 AM
Unknown Object (File)
Oct 5 2024, 1:42 AM
Unknown Object (File)
Oct 5 2024, 1:42 AM
Unknown Object (File)
Oct 5 2024, 1:40 AM

Details

Summary

It seems that this function is not used anywhere, so I am hoping it's okay to switch to an async function.

(The reason for this is that I want to use importJSON so I can later refactor all configs so they can be specified by environmental variables in the Docker context.)

Depends on D4024

Test Plan

Call getOlmConfig at the end of generateOlmConfig and make sure it works. Diff: https://gist.github.com/Ashoat/3b4ad12b59c54847f8f277914d12837e

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

tomek added 1 blocking reviewer(s): marcin.

Updates after feedback in D4024 to use generics instead of any

@marcinwasowicz, ping on this – would love advice on the test plan!

marcin added inline comments.
keyserver/src/utils/olm-utils.js
13 ↗(On Diff #12699)

I am only confused about the path here - in previous version we imported olmConfig from '../..' now we import directly from secrets directory. When it comes to test plan I would temporarily modify generate-olm-config script to use this function after it generates config to compare the content. I can do it on my own and share results as a comment for this diff.

This revision is now accepted and ready to land.May 17 2022, 7:11 AM
keyserver/src/utils/olm-utils.js
13 ↗(On Diff #12699)

I am only confused about the path here - in previous version we imported olmConfig from '../..' now we import directly from secrets directory

See the parent D4024 – the importJSON function prepends this path.

When it comes to test plan I would temporarily modify generate-olm-config script to use this function after it generates config to compare the content

Awesome, thanks!!

I can do it on my own and share results as a comment for this diff.

Thanks for offering, but I think I can handle it... I still need to test D4024 anyways...

Confirming that I tested and this works