-
Notifications
You must be signed in to change notification settings - Fork 323
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
UI service MVP #95
UI service MVP #95
Conversation
@damienkilgannon This is an awesome start, great job! Here is what I have done:
I don't think the 404 is the issue here, but perhaps it is.
Regardless, I think we should work on this with the goal of it being released with v1.3, given that Milestone 1.2 is so close. I would like to actually merge this into a new branch Off the top of my head I can think of the following:
From there, you/I can work and make PR's onto the Thoughts? Really nice initial work! |
Sounds, good ... i would think that error is as a result of making request to the rest service at the wrong IP:PORT. The ui is running javascript in your browser which makes GET/POST requests to the rest service, its defaults to localhost and as the ui is running on your browser (not the same host as the rest service) it fails. Need to change to edit ui/static/angular_settings.js to reflect the IP of the virtual machine which the rest service is running on. |
Do you have any suggestions for the 404 error? I can hit EDIT: In the console the following error occurs:
EDIT 2: Everything returns |
rest/rest_service.py
Outdated
@@ -142,6 +143,7 @@ def __init__(self, settings_name): | |||
self.wrapper = SettingsWrapper() | |||
self.logger = None | |||
self.app = Flask(__name__) | |||
CORS(self.app) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@madisonb the CORS module adds support for cross origin requests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the 'No 'Access-Control-Allow-Origin'' should be not be an issue as the CORS modules adds the required HTTP headers to the HTTP messages to allow cross origin requests.
Following your second EDIT .... are you still getting that error?
@madisonb if you have a look at the above screenshot you will see that my client browser connecting to the UI service, note that the request to the REST service from the UI is not via 'localhost' but the IP of the vagrant machine at 'http://192.168.33.99:5343/'. |
@madisonb does that make sense? I think as the IP of the Vagrant machine is set in the 'Vagrantfile' as 192.168.33.99, I should change the default from localhost -> 192.168.33.99. |
@damienkilgannon ah ok, I typically just use docker or run the scripts on my host os in order to test/debug stuff. Can you check to see if the problem works with the following setup (and how do we resolve it)?
This is my setup which ends up complaining. |
I will try with that setup later this evening and see how it can be fixed |
@madisonb I have replicated the issue and its a CORS problem but unsure why its on issue with docker environment on not vagrant. Will revert back when I have fix. |
@damienkilgannon I haven't forgotten about this, would like to get the CORS issue fixed and then can merge this into a |
@madisonb never got a chance to resolve the problem .... I have time this weekend so going to look at it now. |
@madisonb so I made some changes, created a route decorator for the rest service which adds the CORS headers to response in a better way than I had set up before. However that wasn't the problem. The reason it doesn't work with docker is due to the fact that docker pulls the 'rest service' image from the repo which doesn't have the changes I added to allow cross origin requests. For you to run it locally you need to run the rest service and the UI service as they are in my pull request. Does that make sense? |
Sorry didn't mean to create that merge request! |
@madisonb not sure what is causing two of the builds to fail, can you have a look? |
@madisonb I just ran that build job again and all passed. What else would you like me to add in before starting a new branch and merging the pull request into that branch? Is it your plan to have a separate branch for the UI ? |
@damienkilgannon My plan of attack is as follows:
I have been working hard on wrapping up milestone |
Sounds good with me. I will continue working on the PR for the time been. |
Closing as I have merged this branch into a new Please see #116 for remaining checklist to get the UI merged into |
Pull request to provide the first steps to satisfying #25.
A simple angularjs + Flask UI provides a UI to the user to check status of the Scrapy Cluster and submit crawl requests easily.
The status checks and crawl requests are provided via the rest service.