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

Create Status.json #483

Merged
merged 8 commits into from
Apr 15, 2019
Merged

Create Status.json #483

merged 8 commits into from
Apr 15, 2019

Conversation

sashadev-sky
Copy link
Member

@sashadev-sky sashadev-sky commented Apr 3, 2019

References #326

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

  • PR is descriptively titled 📑 and links the original issue above 🔗
  • tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with rake test
  • code is in uniquely-named feature branch and has no merge conflicts 📁
  • screenshots/GIFs are attached 📎 in case of UI updation
  • ask @publiclab/reviewers for help, in a comment below

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software

Thanks!

@sashadev-sky
Copy link
Member Author

@icarito @jywarren I created an endpoint here exports/:id/status and made just a button to show an example of it connecting to the UI... it just updates to the status message once:

show-status

You wanted this to be saved as a local JSON file now?

Here is an example string of the full JSON it returns:

{"bands_string":"","cm_per_pixel":5.0,"created_at":"2019-04-03T20:49:39Z","export_type":"normal","geotiff":false,"height":null,"id":1,"jpg":false,"map_id":1,"size":null,"status":"warping 2 of 2","tms":false,"updated_at":"2019-04-03T21:09:38Z","user_id":1,"width":null,"zip":false}

Note user_id is included above, so that covers the usernames @jywarren was asking about

@sashadev-sky
Copy link
Member Author

(will remove the console.logs when it should be merged)

@jywarren
Copy link
Member

jywarren commented Apr 5, 2019 via email

@sashadev-sky
Copy link
Member Author

sashadev-sky commented Apr 6, 2019

@jywarren Ok this is updated! I'll clean up the code and resolve merge conflict if / when you think this is ready to merge

also in my last PR, for now updated the endpoint to mirror the old progress endpoint: export/status/:id

@sashadev-sky
Copy link
Member Author

@jywarren what is my next tasks after this?

@jywarren
Copy link
Member

jywarren commented Apr 9, 2019

oh wow, does this work now? Can you update the screenshot? Awesome, it looks good!

@jywarren
Copy link
Member

jywarren commented Apr 9, 2019

I think next step will be to allow for it to have a remote status.json file. So, we:

  1. add a remote_url column to Exports
  2. check if it exists
  3. if it does, we redirect requests to status to the remote URL (which'll be our external exporter)
  4. we think of how the client may send a remote URL up to the MapKnitter Rails app for storage?

@jywarren
Copy link
Member

jywarren commented Apr 9, 2019

And then, start to hook up the multiple image select to a remote exporter, if @tech4GT or @icarito are ready to receive a request soon? Even if it's not totally ready?

@sashadev-sky
Copy link
Member Author

@jywarren ok you want this in this PR or a new one?

@sashadev-sky
Copy link
Member Author

sashadev-sky commented Apr 9, 2019

@jywarren I overrwrote the exportProgress method like you asked to make it call our new endpoint export/status/:id instead of export/progress/:id. You can see in gif below it makes calls to this endpoint now when you hit Run Export button.

exporter-gifs

@jywarren
Copy link
Member

This is great. We can def. do the work on a remote status.json file in a follow-up. Is this ready to merge then? Awesome work, Sasha!!!

@sashadev-sky
Copy link
Member Author

sashadev-sky commented Apr 11, 2019

@jywarren this failed due to keyserver issue, but it's ready to merge as soon as travis is back up and running!

Just to confirm #413 or Leaflet.DistortableImage#203 is the next step?

@sashadev-sky
Copy link
Member Author

@jywarren update it worked! Please answer my question above too though :) Thank you!!

@jywarren
Copy link
Member

Awesome. Either would be a great next step!

@sashadev-sky
Copy link
Member Author

@jywarren Ok to me seems like #413 is the next logical step so starting on that this week!

chen-robert pushed a commit to chen-robert/mapknitter that referenced this pull request Dec 5, 2019
* Export bug fix

* Working export json

* Update example view

* Clean up code

* Clean up code pt2

* Refactor export progress to show status JSON

* Update demo view

* Clean up comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants