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

FEATURE: Allow route resolving of disabled nodes #4334

Closed
wants to merge 3 commits into from

Conversation

dlubitz
Copy link
Contributor

@dlubitz dlubitz commented Jun 13, 2023

Adds the ability to allow route resolving of disabled nodes.

This is needed for the redirect handling, to create URLs for given nodes, even if they are disabled at the moment.

@bwaidelich
Copy link
Member

As written on Slack:

I struggle to extend our core API for a feature that I don't even stand behind

My personal short term goal would be to not create redirects for disabled nodes for now

adding a new global request parameter that allows routing for disabled nodes could be a pandoras box.
e.g. we were thinking about hard constraints for URIs (globally avoid duplicates at any time etc) and this change
would make that more complex – for example if you want to reuse a URI that was used by a disabled node

@mhsdesign
Copy link
Member

mhsdesign commented Jun 20, 2023

Hmm the suggestion with route Paramus looks a bit odd (due to this odd feature/escape hatch)

I would rather have some low level internal resolver which doesn’t care about disabled and use this in your place directly? Wouldn’t this work?

@dlubitz
Copy link
Contributor Author

dlubitz commented Jun 21, 2023

Could you please explain "low level internal resolver"? ATM everything is bound to the EventSourcedFrontendNodeRoutePartHandler an not reusable. Only way might be to move this out. But TBH I don't know if this will work, as there might some magic in the other RoutePartHandler as well.

@bwaidelich
Copy link
Member

Could you please explain "low level internal resolver"?

I guess, this is about the same topic that we talked about before:
Extracting parts of the resolving logic from the FrontendNodeRoutePartHandler such that it can be used in other places (and allows for hooking into the mechanism in order to extend it)

But after thinking about it (once again) I think this is a dangerous path to follow.. IMO there should be only a single way to build and resolve URIs and either we support routing to disabled nodes or not

@dlubitz
Copy link
Contributor Author

dlubitz commented Jun 23, 2023

Closed in favor of #4363

@dlubitz dlubitz closed this Jun 23, 2023
@dlubitz dlubitz deleted the 90/feature-show-disabled-nodes branch July 28, 2023 14:09
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.

None yet

3 participants