feat(platform): Support @Searchable + @Relationship Annotations for Timeseries Aspects #6455
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jjoyce0510
commented
Nov 16, 2022
| entitySpec = _entityRegistry.getEntitySpec(event.getEntityType()); | ||
| } catch (IllegalArgumentException e) { | ||
| log.error("Error while processing entity type {}: {}", event.getEntityType(), e.toString()); | ||
| public void invoke(@Nonnull final MetadataChangeLog event) { |
There was a problem hiding this comment.
This is mostly just organizing code.
There was a problem hiding this comment.
- adding javadocs.
jjoyce0510
commented
Nov 16, 2022
|
|
||
| // Step 0. If the aspect is timeseries, add to its timeseries index. | ||
| if (aspectSpec.isTimeseries()) { | ||
| updateTimeseriesFields(event.getEntityType(), event.getAspectName(), urn, aspect, aspectSpec, |
There was a problem hiding this comment.
Here's the important part: Only update timeseries index if its a time series aspect. Do everything else either way.
jjoyce0510
changed the title
Unit Test Results (build & test)621 tests - 1 617 ✔️ - 1 16m 0s ⏱️ -3s Results for commit fd93883. ± Comparison against base commit 1fe0f01. This pull request removes 7 and adds 6 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
chriscollins3456
approved these changes
Nov 17, 2022
Comment on lines
12
to
40
| record DatasetProfile includes TimeseriesAspectBase { | ||
| /** | ||
| * The total number of rows | ||
| */ | ||
| @Searchable = { | ||
| "fieldType": "COUNT" | ||
| } | ||
| rowCount: optional long | ||
|
|
||
| /** | ||
| * The total number of columns (or schema fields) | ||
| */ | ||
| @Searchable = { | ||
| "fieldType": "COUNT" | ||
| } | ||
| columnCount: optional long | ||
|
|
||
| /** | ||
| * Profiles for each column (or schema field) | ||
| */ | ||
| fieldProfiles: optional array[DatasetFieldProfile] | ||
|
|
||
| /** | ||
| * Storage size in bytes | ||
| */ | ||
| @Searchable = { | ||
| "fieldType": "COUNT" | ||
| } | ||
| sizeInBytes: optional long |
There was a problem hiding this comment.
is it just the diff view or does the indentation of your additions seem off here?
There was a problem hiding this comment.
whoa no this seems off
shirshanka
added
the
platform
david-leifker
approved these changes
Nov 28, 2022
RyanHolstien
approved these changes
Nov 28, 2022
cccs-Dustin
pushed a commit
to CybercentreCanada/datahub
that referenced
this pull request
Feb 1, 2023
…imeseries Aspects (datahub-project#6455)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
In this PR, we add support for marking fields in a Timeseries aspect as @searchable. This allows us to easily index the most recent value of a particular field. We also add indexing to the following fields:
We do this by simply indexing the Searchable fields during normal MCL processing. On each new Timeseries aspect, the previous value will be overwritten.
What does this enable?
This allows us to easily filter by the fields in a Timeseries aspect.
Status
Ready for review. Validated everything manually as well.
Checklist