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

Data extension service not using CORS #3672

Closed
gitonthescene opened this issue Feb 26, 2021 · 2 comments · Fixed by #3710
Closed

Data extension service not using CORS #3672

gitonthescene opened this issue Feb 26, 2021 · 2 comments · Fixed by #3710
Labels
reconciliation Related to the reconciliation operations and other features Type: Bug Issues related to software defects or unexpected behavior, which require resolution.
Milestone

Comments

@gitonthescene
Copy link
Contributor

gitonthescene commented Feb 26, 2021

To Reproduce

Steps to reproduce the behavior:

  1. Implement a data reconciliation service with the data extension service specified in the service manifest.
  2. Open the browser web console.
  3. Finally, try to "Add columns from reconciled values"

Current Results

Currently you get back a blank form with no properties for selection. I think given the bug from this issue it probably should have shown an error to the user. But when you look in the web console, you can see that the request was using JSONP.

GET http://127.0.0.1:5000/properties?type=&callback=jQuery1111022504064285953862_1613364041041&_=1613364041042

Expected Behavior

Similar to reconciliation, I would have expected it to try to use CORS first and fallback to JSONP.

Screenshots

Versions

  • Operating System:
  • Browser Version:
  • JRE or JDK Version:
  • OpenRefine:

Datasets

Additional context

I can provide a sample service demonstrating this bug if you like. Please let me know. I think the code making this request is here. It looks like recon calls ajax directly rather than with the jQuery shorthand.

For clarity, this is using OpenRefine 3.4.1 on a Mac with FF 84.0.1.

[EDIT: You can see the use of JSONP from the web console using the Wikidata reconciliation service, so no need to implement your own service.]

@gitonthescene gitonthescene added Type: Bug Issues related to software defects or unexpected behavior, which require resolution. Status: Pending Review Indicates that the issue or pull request is awaiting review by project maintainers or collaborators labels Feb 26, 2021
@gitonthescene
Copy link
Contributor Author

gitonthescene commented Feb 26, 2021

Also it looks like the data property proposal request is coming through the data extension service end point, but the extend request is coming through the reconciliation end point. Is that right? Or maybe it's the same service_url but using the /reconcile end point instead of the service_path for property proposals? I can't tell because the base url is the same for both in my sample service. In any event, this isn't clear from the spec.

$ poetry run csv-reconcile --init-db sample/currreps.tsv item itemLabel --config=sample/sample.cfg
Initialized the database.
 * Serving Flask app "csv-reconcile" (lazy loading)
 * Environment: production
   WARNING: This is a development server. Do not use it in a production deployment.
   Use a production WSGI server instead.
 * Debug mode: off
 * Running on http://127.0.0.1:5000/ (Press CTRL+C to quit)
127.0.0.1 - - [26/Feb/2021 09:22:35] "GET /properties?type=&callback=jQuery1111022504064285953862_1613364041045&_=1613364041046 HTTP/1.1" 200 -
[2021-02-26 09:22:41,415] INFO in __init__: ImmutableMultiDict([('extend', '{"ids":["Q946606","Q26436159","Q18978140","Q58754391","Q289317"],"properties":[{"id":"partyLabel"}]}')])
127.0.0.1 - - [26/Feb/2021 09:22:41] "POST /reconcile HTTP/1.1" 200 -

@gitonthescene
Copy link
Contributor Author

I think this line shows that the data extension extend request is going to the reconciliation service.

My reading of the spec is that these requests are going to the service_url specified in the extend portion of the service manifest and I assume the same service_path as well. Either way the intended endpoint should probably be made more explicit. I’m not sure what the intention here is. Perhaps it’s better to refer to the “data extension service of the reconciliation server”? But then it’s not clear why property proposals are allowed to come from a separate service_url.

@wetneb wetneb added reconciliation Related to the reconciliation operations and other features and removed Status: Pending Review Indicates that the issue or pull request is awaiting review by project maintainers or collaborators labels Mar 12, 2021
@wetneb wetneb added this to the 3.5 milestone Mar 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
reconciliation Related to the reconciliation operations and other features Type: Bug Issues related to software defects or unexpected behavior, which require resolution.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants