-
Notifications
You must be signed in to change notification settings - Fork 117
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[front] Shadow read content nodes from core
and compute diff
#10061
Conversation
77dc6f3
to
83fdfe2
Compare
83fdfe2
to
a603a0f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking about it, the endstate will be a single call, not differentiating whether the datasource is static or managed right? so I think the best would be to shadow read at the toplevel function; wdyt?
front/lib/api/data_source_view.ts
Outdated
|
||
// shadow read from core | ||
const coreContentNodesRes = | ||
await getContentNodesForManagedDataSourceViewFromCore( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about turning that call into getContentNodesForDatasourceViewCore
doing both static & managed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes indeed, next step would have been to move anyway!
…odesForDataSourceViewFromCore and handle static data sources altogether
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great 👍 Left a suggestion
"[CoreNodes] Could not fetch content nodes from core" | ||
); | ||
} else if (coreContentNodesRes.isOk()) { | ||
if (coreContentNodesRes.value.total !== contentNodesResult.total) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: core defaults to 100 results while connectors is not paginated (returns everything)
I suggest switching to 250 in searchNodes from the core call for now to limit the logs on count mismatch that are just related to that (since it's not actually something to fix)
We are probably going to get a lot of false positives, notably on |
* add shadow read and content nodes comparison * add a log of extra core nodes * fix the diff for arrays (parents) * pass the parentsIn when no internalIds are passed * turn getContentNodesForManagedDataSourceViewFromCore into getContentNodesForDataSourceViewFromCore and handle static data sources altogether * 📝 * bump the limit to 250 on searchNodse
Description
Risk
Deploy Plan