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

ngtests: test complex delegation graphs #1643

Closed
sechkova opened this issue Oct 27, 2021 · 2 comments · Fixed by #1689
Closed

ngtests: test complex delegation graphs #1643

sechkova opened this issue Oct 27, 2021 · 2 comments · Fixed by #1689
Assignees
Labels
backlog Issues to address with priority for current development goals testing
Milestone

Comments

@sechkova
Copy link
Contributor

Description of issue or feature request:

Add tests dedicated to forming complex delegation graphs:

  • Cyclic, diamond, big graphs
  • Delegated hashed bins (RepoSimulator doesn’t support them yet)

Explore formats for describing delegations (idea: Graphviz dot format)

@sechkova sechkova added testing backlog Issues to address with priority for current development goals labels Oct 27, 2021
@sechkova sechkova mentioned this issue Nov 22, 2021
3 tasks
@sechkova
Copy link
Contributor Author

Since @lukpueh is back :)) and together with @jku are working on a repository design, I'd like to resurrect an old issue #589.

How does, according to you, the new code should treat delegations of the type

A -> C
B -> C

given that A and B may delegate different paths, use different thresholds etc.
Does it make sense for practical use cases? Should we simplify our life and just forbid it? Should we support it?
We've just merged #1528 which means that the client will skip C in this example, the second time it reaches it through B (if the pathpattern is matched in both).

@sechkova sechkova self-assigned this Nov 22, 2021
@sechkova sechkova added this to the Sprint 12 milestone Nov 22, 2021
@jku
Copy link
Member

jku commented Nov 23, 2021

My naive thinking is that for the client 1528 is fine: in a specific target path lookup we don't want to traverse into C again if we've already visited it (results should be same), but generally speaking there's no harm in allowing C from being accessible through multiple routes: A different target path lookup might only go "targets->B->C" and I don't see why we'd want to prevent that just because A also happens to delegate something unrelated to C.

The repository application might want to enforce simple graphs for the simplicity, but that sounds like an application decision.

@sechkova sechkova modified the milestones: Sprint 12, Sprint 13 Nov 24, 2021
@sechkova sechkova modified the milestones: Sprint 13, Sprint 14 Dec 8, 2021
@jku jku closed this as completed in #1689 Dec 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog Issues to address with priority for current development goals testing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants