Skip to content
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

HPCC-33540 Persist Metrics Lineage #19611

Open
wants to merge 1 commit into
base: candidate-9.2.x
Choose a base branch
from

Conversation

GordonSmith
Copy link
Member

@GordonSmith GordonSmith commented Mar 12, 2025

Both selection and lineage selection should be persisted

Type of change:

  • This change is a bug fix (non-breaking change which fixes an issue).
  • This change is a new feature (non-breaking change which adds functionality).
  • This change improves the code (refactor or other change that does not change the functionality)
  • This change fixes warnings (the fix does not alter the functionality or the generated code)
  • This change is a breaking change (fix or feature that will cause existing behavior to change).
  • This change alters the query API (existing queries will have to be recompiled)

Checklist:

  • My code follows the code style of this project.
    • My code does not create any new warnings from compiler, build system, or lint.
  • The commit message is properly formatted and free of typos.
    • The commit message title makes sense in a changelog, by itself.
    • The commit is signed.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly, or...
    • I have created a JIRA ticket to update the documentation.
    • Any new interfaces or exported functions are appropriately commented.
  • I have read the CONTRIBUTORS document.
  • The change has been fully tested:
    • I have added tests to cover my changes.
    • All new and existing tests passed.
    • I have checked that this change does not introduce memory leaks.
    • I have used Valgrind or similar tools to check for potential issues.
  • I have given due consideration to all of the following potential concerns:
    • Scalability
    • Performance
    • Security
    • Thread-safety
    • Cloud-compatibility
    • Premature optimization
    • Existing deployed queries will not be broken
    • This change fixes the problem, not just the symptom
    • The target branch of this pull request is appropriate for such a change.
  • There are no similar instances of the same problem that should be addressed
    • I have addressed them here
    • I have raised JIRA issues to address them separately
  • This is a user interface / front-end modification
    • I have tested my changes in multiple modern browsers
    • The component(s) render as expected

Smoketest:

  • Send notifications about my Pull Request position in Smoketest queue.
  • Test my draft Pull Request.

Testing:

@GordonSmith GordonSmith requested review from jeclrsg and Copilot March 12, 2025 13:43
Copy link

Jira Issue: https://hpccsystems.atlassian.net//browse/HPCC-33540

Jirabot Action Result:
Workflow Transition To: Merge Pending
Updated PR

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request implements persistence for both metrics selection and lineage selection. Key changes include updating the components and routes to accept and parse separate lineage and selection parameters, refactoring functions in the metric graph utilities, and modifying various UI components and URL construction to support the new state structure.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
esp/src/src-react/components/MetricsGraph.tsx Refactored to update how metric graph data is built and returned, notably adjusting selection and lineage persistence.
esp/src/src-react/routes.tsx Modified route definitions to split state parameters into lineageSelection and selection arrays.
esp/src/src-react/components/QueryMetrics.tsx Updated Metrics component usage by splitting comma‐separated selections into arrays.
esp/src/src-react/layouts/HpccJSAdapter.tsx Added onReady support for the HpccJSComponent to better control widget initialization.
esp/src/src-react/util/metricGraph.ts Refactored vertexStatus (and related functions) and updated graphTpl to work with new parameter types.
esp/src/src-react/components/Metrics.tsx Adjusted URL push logic and component state to use the new lineageSelection and selection array structure.
esp/src/src-react/components/WorkunitDetails.tsx Updated metrics state structure to include lineageSelection and selection in an object.
esp/src/src-react/components/QueryDetails.tsx Modified queryParams interface to support lineageSelection alongside metricsSelection.
Comments suppressed due to low confidence (2)

esp/src/src-react/components/Metrics.tsx:33

  • The new pushSelectionUrl function now builds URLs using both a lineageSelection and an array for selection. Ensure that the resulting URL structure meets navigation requirements and that all caller components have been updated to pass the correct types.
const pushSelectionUrl = (parentUrl: string, lineageSelection?: string, selection?: string[]) => {

esp/src/src-react/routes.tsx:37

  • The updated route now passes a state object with a nested structure for metrics (including lineageSelection and an array for selection). Please confirm that all downstream consumers of this state are updated to correctly extract and utilize these fields.
return <_.WorkunitDetails wuid={params.Wuid as string} parentUrl={params.parentUrl as string} fullscreen={...} tab={params.Tab as string} state={{ [params.Tab as string]: { lineageSelection: params.State, selection: (params.State2 as string).split(",") } }} queryParams={{ ... }} />;

@@ -223,21 +223,23 @@ export class MetricGraph extends Graph2<IScopeEx, IScopeEdge, IScopeEx> {
v.Label || v.id;
}

vertexStatus(v: IScopeEx): string {
const retVal: Array<ScopeStatus | ExceptionStatus> = [];
vertexStatus(v: IScopeEx): ScopeStatus {
Copy link
Preview

Copilot AI Mar 12, 2025

Choose a reason for hiding this comment

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

The refactored vertexStatus function now returns only a single status ('completed', 'started', or 'unknown') instead of aggregating multiple statuses. Please verify that this behavior correctly reflects the intended state of a vertex and does not omit important exception status details.

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
Copy link
Member Author

Choose a reason for hiding this comment

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

This is correct, the aggregated string is returned in vertexClass

Both selection and lineage selection should be persisted

Signed-off-by: Gordon Smith <GordonJSmith@gmail.com>
@GordonSmith GordonSmith force-pushed the HPCC-33540-METRICS_VIEW_PERSIST branch from 80a757d to 2cba7a6 Compare March 12, 2025 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants