Skip to content

Data flow: Add ConfigSig::accessPathLimit - #15867

Merged
hvitved merged 4 commits into
github:mainfrom
hvitved:dataflow/ap-limit
Mar 12, 2024
Merged

hvitved merged 4 commits into
github:mainfrom
hvitved:dataflow/ap-limit

Conversation

@hvitved

@hvitved hvitved commented Mar 11, 2024

Copy link
Copy Markdown
Contributor

Adds an option to control the access path limit on a per-configuration basis. Needed for #15866.

@hvitved hvitved added the no-change-note-required This PR does not need a change note label Mar 12, 2024
@hvitved
hvitved marked this pull request as ready for review March 12, 2024 07:47
@hvitved
hvitved requested review from a team as code owners March 12, 2024 07:47
atorralba
atorralba previously approved these changes Mar 12, 2024

@atorralba atorralba left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As I understand it, we already had a default access path limit of 5, so this shouldn't cause any changes in results.

@hvitved

hvitved commented Mar 12, 2024

Copy link
Copy Markdown
Contributor Author

As I understand it, we already had a default access path limit of 5, so this shouldn't cause any changes in results.

Correct, no result changes are expected.

michaelnebel
michaelnebel previously approved these changes Mar 12, 2024

@michaelnebel michaelnebel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

C# LGTM!

@aschackmull aschackmull left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think you missed the reference to accessPathLimit in partial flow.

@hvitved
hvitved force-pushed the dataflow/ap-limit branch 2 times, most recently from 5888236 to 1161ca2 Compare March 12, 2024 11:03
owen-mc
owen-mc previously approved these changes Mar 12, 2024

@owen-mc owen-mc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Go 👍🏻

@aschackmull aschackmull left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM now

Sign up for free to 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.

5 participants