Peco 273: Add support for connector to be run in browser#53
Conversation
Signed-off-by: nithinkdb <nithin.krishnamurthi@databricks.com>
Signed-off-by: nithinkdb <nithin.krishnamurthi@databricks.com>
Signed-off-by: nithinkdb <nithin.krishnamurthi@databricks.com>
Signed-off-by: nithinkdb <nithin.krishnamurthi@databricks.com>
Signed-off-by: nithinkdb <nithin.krishnamurthi@databricks.com>
Signed-off-by: nithinkdb <nithin.krishnamurthi@databricks.com>
Signed-off-by: nithinkdb <nithin.krishnamurthi@databricks.com>
Signed-off-by: nithinkdb <nithin.krishnamurthi@databricks.com>
Signed-off-by: nithinkdb <nithin.krishnamurthi@databricks.com>
Signed-off-by: nithinkdb <nithin.krishnamurthi@databricks.com>
Signed-off-by: nithinkdb <nithin.krishnamurthi@databricks.com>
Signed-off-by: nithinkdb <nithin.krishnamurthi@databricks.com>
Signed-off-by: nithinkdb <nithin.krishnamurthi@databricks.com>
Signed-off-by: nithinkdb <nithin.krishnamurthi@databricks.com>
moderakh
left a comment
There was a problem hiding this comment.
For this type of work, a small design doc / one decision doc would be helpful to evaluate alternatives and decide what to do.
user agent cannot be set in browsers by the sdk, one common typical solution for this to work in browsers is to use a second header. for example azure java-script sdks use x-ms-useragent for browser support.
see this for more detail: Azure/azure-sdk-for-js#6530
@nithinkdb did we evaluate if we can utilize this technique and coordinate with thrift server to use an additional header as user-agent?
also please note support for browser is good to have but not a GA blocker.
We did evaluate this, and it is unfortunately insufficient. While we do need to change the user-agent header, we will still run into CORS issues unless we either route through a proxy, or the backend team starts sending access-control-allow-origin headers. This is technically doable though, and we should consider escalating to the correct team. |
|
is this still an active/relevant PR? if not @nithinkdb could you please close the PR? |
|
closing this as not relevant right now. |
This PR enables XHR connections in the cases where our connector is run from the browser. A couple things to note:
https://github.com/nithinkdb/connectorjstest
I haven't made it public yet in order to avoid confusion, given that none of this code will actually work until the PR on the thrift side completes. In the meantime, though, I think it'd be good to merge this helper code.
Worst case scenario, I have a fork of the thrift repo with the correct code, but that seems like overkill/unnecessary.
Note: This is not a GA blocker.