Skip to content

refactor: Correlations - #1115

Merged
nitisht merged 18 commits into
parseablehq:mainfrom
de-sh:correlations
Jan 28, 2025
Merged

refactor: Correlations#1115
nitisht merged 18 commits into
parseablehq:mainfrom
de-sh:correlations

Conversation

@de-sh

@de-shde-sh commented Jan 17, 2025

Copy link
Copy Markdown
Contributor

Fixes #XXXX.

Description

  • Use Deref, enables changing self.0 ~> self.
  • Use HashMap to enable quick retreival.
  • Encapsulate mutations into a single method.

This PR has:

  • been tested to ensure log ingestion and log query works.
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added documentation for new or modified features or behaviors.

@coveralls

coveralls commented Jan 17, 2025

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 12994526875

Details

  • 0 of 137(0.0%) changed or added relevant lines in 2 files are covered.
  • 7 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.02%) to 12.753%

Changes Missing CoverageCovered LinesChanged/Added Lines%
src/handlers/http/correlation.rs0290.0%
src/correlation.rs01080.0%
Files with Coverage ReductionNew Missed Lines%
src/handlers/http/correlation.rs10.0%
src/correlation.rs60.0%
TotalsCoverage Status
Change from base Build 12991591562:0.02%
Covered Lines:2443
Relevant Lines:19157

💛 - Coveralls

@nikhilsinhaparseable

Copy link
Copy Markdown
Member

@de-sh while validating the PR, i found that the id of the correlation is not loaded at the server start but the json has the id.
image
image

can you please check.

@nikhilsinhaparseablenikhilsinhaparseable left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

@nitisht
nitisht merged commit 12c507c into parseablehq:mainJan 28, 2025
@de-sh
de-sh deleted the correlations branch January 28, 2025 09:43
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@de-sh@coveralls@nikhilsinhaparseable@nitisht