perf: resolve jsonurls while merging reports #499
Conversation
KuznetsovRoman
force-pushed
the
HERMIONE-1138.resolve_jsonuls
branch
2 times, most recently
from
aa26369 to
6477d4d
Compare
KuznetsovRoman
changed the title
| return resolvedUrls; | ||
| } | ||
|
|
||
| async function tryResolveUrl(url) { |
There was a problem hiding this comment.
Рекурсивно проходит по всем вложенным .json базам. Если возникает ошибка при получении данных, возвращаем саму json-базу. Помимо "ничего не сломать", фоллбэк нужен для драфт отчетов
| results.forEach(({jsonUrls, dbUrls}) => { | ||
| resolvedUrls.push(...jsonUrls.concat(dbUrls)); | ||
| }); | ||
|
|
||
| return resolvedUrls; |
There was a problem hiding this comment.
Все urls вставляются в один массив, потому что это позволяет не ломать API слияния отчетов
|
|
||
| responses.forEach(response => { | ||
| dbUrls.push(...response.dbUrls); | ||
| jsonUrls.push(...response.jsonUrls); |
There was a problem hiding this comment.
для чего нам здесь накапливать json-урлы? просто на всякий случай, чтобы понять откуда базы приехали?
There was a problem hiding this comment.
Нет, тут логика такая:
в responses хранятся dbUrls и jsonUrls, и этот responses образуется как результат выполнения этой же функции
Эта же функция берет url и если это jsonUrl, пытается ее развернуть. Результаты .db складывает, результаты .json пытается развернуть. Если не получилось по какой-то причине, возвращаем этот json-урл. Собственно, там не изначальные json-урлы, а только те, по которым не получилось скачать сами json-ы. В результате, если сеть работает, а запросы шлются нормально - эти jsonUrls будут пустыми. Если сеть не работает, или запрос выкинул ошибку - jsonUrls будет равен исходному
| } catch (e) { | ||
| jsonUrls.push(url); | ||
| } | ||
| } |
There was a problem hiding this comment.
у нас разве где-то есть проверка на то, что мы можем передать только путы формата json/db? сейчас у тебя нет обработки на такой случай и мы вернем пустые массивы. Это вроде и норм, но ругнуться все равно на это нужно
test/unit/lib/merge-reports/index.js
Outdated
| const destination = 'dest-report/path'; | ||
|
|
||
| axiosStub.get.withArgs('src-report/path-1.json').resolves({data: {jsonUrls: ['src-report/path-2.json'], dbUrls: ['path-3.db']}}); | ||
| axiosStub.get.withArgs('src-report/path-2.json').resolves({data: {jsonUrls: ['src-report/path-4.json'], dbUrls: ['path-5.db']}}); |
There was a problem hiding this comment.
если мы в тесте проверяем именно резолв json-файлов, но и нумерацию нужно сделать последовательную (path-1.json, path-2.json, path-3.json). А то сейчас путаешься, пытаясь уследить за последовательностью цифр, которые смешиваются json+db
и ты здесь не проверяешь кейс, когда, у тебя в jsonUrls больше одного элемента. чет типа такого:
axiosStub.get.withArgs('path-1.json').resolves({data: {jsonUrls: ['path-2.json', 'path-3.json'], dbUrls: ['path-1.db']}});
axiosStub.get.withArgs('path-2.json').resolves({data: {jsonUrls: [], dbUrls: ['path-2.db']}});
axiosStub.get.withArgs('path-3.json').resolves({data: {jsonUrls: [], dbUrls: ['path-3.db']}});
KuznetsovRoman
force-pushed
the
HERMIONE-1138.resolve_jsonuls
branch
from
6477d4d to
6db2183
Compare
KuznetsovRoman
force-pushed
the
HERMIONE-1138.resolve_jsonuls
branch
from
6db2183 to
7d9150f
Compare
Что сделано
Если скачанный json с базами данных содержит ссылки на другие json-ы, на этапе слияния отчетов скачиваем их тоже, чтобы получить их содержимое. После чего ссылки на базы данных кладем в список ссылок на базы данных, а с ссылками на json делаем рекурсивно так же. Если json скачать не удалось, фоллбэкаем на текущую схему (кладем его в список json-ов)