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

Request resolution at data store level is ambiguous, may result in wrong route taken #3062

Closed
vladsud opened this issue Aug 5, 2020 · 2 comments
Assignees
Labels
bug Something isn't working focus Items that engineers are focusing on now, but may not have any (coding) outcome in current milestone status: stale
Milestone

Comments

@vladsud
Copy link
Contributor

vladsud commented Aug 5, 2020

Also naming needs to be revisited. I.e. request() -> resolveHandle() where it makes sense.

Discussion:
#2969 (comment)

Core of the problem:

[ChumpChief]
I think I'm ok with this, but I do feel a little funny about calling this "request" -- the purpose/mission of this is pretty different from our current "request"s, maybe it would be more appropriate to use handle-specific names all the way through this flow?

I.e. I got tripped up for a bit on why runtime.request vs. runtime.IFluidHandleContext.request was a significant distinction to be made -- it's more than just asking someone else to respond to the request, it's also a different semantic about the thing you're requesting.

[vladsud]
I agree with you, but I'm a bit afraid to touch this in this PR.
In particular, there is duality of FluidDataStoreRuntime.request(). Calls to it can come from resolving a handle to a DDS. But they also can come as result of handling external URIs. i.e. external request /createTable?rows=2 can be routed to "root table factory" data store in the form of "/?rows=2" request. Data stores have to handle both types of requests.

That's the reason I started this PR with terminology of "internal" & "external" requests, not specifying (yet) on what types of request they can be.
Note that we have the following problem: it's not obvious that IFluidObject's handle implementation can't have (relative) uri="root" - everything will be fine until such handle resolves to root DDS in same data store. The "fix" here is not to have both request() & resolveHandle() APIs on data store (and other layers), but rather encode URIs such that they uniquely point to which route to take. I.e.
/defaultComponent/DDSs/root (handle resolution)
/defaultComponent/objects/root (handle resolution, will map to /root request to custom handler)
/defaultComponent/?rows=2 (request from within container, will map to /?rows=2 to custom handler)

First two can (in future) go through resolveHandle() API route.
The last two need registered data store handler's help, and they can collide (same namespace), but it's in full control of component developer.

so I'd say for now, I'd prefer to do nothing, or rather rename back resolveHandle to internalResolve().
And then take another look at naming, routing, and figure out a plan to make it better, with back compat in mind (continue to serve existing URIs)

@vladsud vladsud added the bug Something isn't working label Aug 5, 2020
@vladsud vladsud added this to the August 2020 milestone Aug 5, 2020
@vladsud vladsud self-assigned this Aug 5, 2020
@ghost ghost added the triage label Aug 5, 2020
@curtisman curtisman removed the triage label Aug 10, 2020
@vladsud vladsud modified the milestones: August 2020, September 2020 Aug 31, 2020
@vladsud vladsud modified the milestones: September 2020, October 2020 Oct 1, 2020
@vladsud vladsud modified the milestones: October 2020, November 2020 Oct 30, 2020
@vladsud vladsud modified the milestones: November 2020, December 2020 Dec 2, 2020
@vladsud vladsud added the focus Items that engineers are focusing on now, but may not have any (coding) outcome in current milestone label Dec 15, 2020
@vladsud vladsud modified the milestones: January 2021, February 2021 Jan 28, 2021
@curtisman curtisman modified the milestones: February 2021, March 2021 Feb 26, 2021
@vladsud vladsud modified the milestones: March 2021, April 2021 Mar 30, 2021
@vladsud vladsud modified the milestones: April 2021, May 2021 Apr 8, 2021
@vladsud vladsud modified the milestones: May 2021, Next May 21, 2021
@ghost ghost added the status: stale label Nov 18, 2021
@ghost
Copy link

ghost commented Nov 18, 2021

This issue has been automatically marked as stale because it has had no activity for 180 days. It will be closed if no further activity occurs within 8 days of this comment. Thank you for your contributions to Fluid Framework!

@ghost ghost added the status: stale label May 19, 2022
@ghost
Copy link

ghost commented May 19, 2022

This issue has been automatically marked as stale because it has had no activity for 180 days. It will be closed if no further activity occurs within 8 days of this comment. Thank you for your contributions to Fluid Framework!

@ghost ghost closed this as completed May 27, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working focus Items that engineers are focusing on now, but may not have any (coding) outcome in current milestone status: stale
Projects
None yet
Development

No branches or pull requests

3 participants