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

Disable debug metrics for components inside foreach, and for foreach itself. #2404

Merged

Conversation

ptodev
Copy link
Collaborator

@ptodev ptodev commented Jan 15, 2025

Opening a separate PR for @wildum to review the changes before they go on the main dynamic pipelines branch.

@ptodev ptodev requested a review from wildum January 15, 2025 10:39
@ptodev ptodev requested a review from a team as a code owner January 15, 2025 10:39
Copy link
Contributor

@wildum wildum left a comment

Choose a reason for hiding this comment

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

Thanks, looks good overall, I really like the added tests, just a few small comments

internal/runtime/foreach_metrics_test.go Outdated Show resolved Hide resolved
internal/runtime/foreach_test.go Outdated Show resolved Hide resolved
internal/runtime/foreach_test.go Outdated Show resolved Hide resolved
internal/runtime/testdata/foreach_metrics/foreach_2.txtar Outdated Show resolved Hide resolved
internal/runtime/testdata/foreach_metrics/foreach_1.txtar Outdated Show resolved Hide resolved
@ptodev ptodev force-pushed the ptodev/dynamic-pipelines-disable-metrics branch from 5578777 to 6a2db08 Compare January 15, 2025 14:55
@ptodev ptodev merged commit 30766dd into ptodev/dynamic-pipelines Jan 15, 2025
15 checks passed
@ptodev ptodev deleted the ptodev/dynamic-pipelines-disable-metrics branch January 15, 2025 15:26
wildum added a commit that referenced this pull request Feb 12, 2025
* Proposal for dynamic pipelines

* Foreach prototype

* Initial implementation

* wip

* Fixes to summation1 and summation2

* summation1 only sends during run()
* summation2 tracks the sum via metrics instead of an export

* fix foreach run

* foreach uses the value from the collection via the var

* compute an ID for the foreach instances and add tests

* rework foreach txtar tests

* support using modules inside of foreach

* cleanup

* update frontend to use the moduleID of the component instead of the module one

* plug the foreach node to the UI

* fix internal template components link

* update comment in component references

* cleanups

* Disable debug metrics for components inside foreach, and for foreach itself. (#2404)

* Add stability lvl to config blocks (#2441)

* add stability lvl to config blocks

* fix import git test

* Add tests for types other than integers (#2436)

* Add tests for types other than integers

* Minor fixes to string_receiver

* Add a foreach test for maps which contain maps

* Add docs for foreach (#2447)

* Add docs for foreach

* Apply suggestions from code review

Co-authored-by: Clayton Cornell <[email protected]>

* Add a shared experimental_feature snippet

* Addressing PR feedback

* Apply suggestions from code review

Co-authored-by: William Dumont <[email protected]>

---------

Co-authored-by: Clayton Cornell <[email protected]>
Co-authored-by: William Dumont <[email protected]>

* use full hash on foreach instances and fix test

* Add a changelog entry.

* typo

* allow non alphanum strings

* add test for wrong collection type

* added capsule test

* Add more tests for non-alphanumeric strings.

* Apply suggestions from code review

Co-authored-by: Clayton Cornell <[email protected]>

* Apply suggestions from code review

Co-authored-by: Clayton Cornell <[email protected]>

* Add comments regarding the override registry for modules

* Rename hashObject to objectFingerprint

* add comment for the hash function

* add additional detail to the comment

---------

Co-authored-by: William Dumont <[email protected]>
Co-authored-by: Clayton Cornell <[email protected]>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 15, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants