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

Cloud Export integration, saving & display #1192

Merged
merged 57 commits into from
Mar 26, 2020

Conversation

jywarren
Copy link
Member

@jywarren jywarren commented Jan 10, 2020

Fixes #1190 and addresses #1191

This will require some manual testing!

@codeclimate
Copy link

codeclimate bot commented Jan 10, 2020

Code Climate has analyzed commit 9a26a4d and detected 0 issues on this pull request.

View more on Code Climate.

@jywarren jywarren changed the title Export display Cloud Export integration, saving & display Jan 10, 2020
@codecov
Copy link

codecov bot commented Jan 10, 2020

Codecov Report

Merging #1192 into main will not change coverage by %.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1192   +/-   ##
=======================================
  Coverage   73.28%   73.28%           
=======================================
  Files          40       40           
  Lines        1400     1400           
=======================================
  Hits         1026     1026           
  Misses        374      374           
Impacted Files Coverage Δ
app/controllers/export_controller.rb 90.90% <100.00%> (ø)

@jywarren
Copy link
Member Author

I was able to get this running locally but was blocked by a CORS limitation:

Access to XMLHttpRequest at 'http://export.mapknitter.org/export' from origin 'http://localhost:3000' has been blocked by CORS policy: No 'Access-Control-Allow-Origin' header is present on the requested resource.

I'm surprised -- this is working elsewhere. Checking with @icarito on this...

@jywarren
Copy link
Member Author

@jywarren
Copy link
Member Author

Maybe we need to allow different ports?

@jywarren
Copy link
Member Author

@jywarren
Copy link
Member Author

Also having trouble getting it to build at https://jenkins.laboratoriopublico.org/job/Mapknitter-Unstable/

@icarito
Copy link
Member

icarito commented Jan 11, 2020

Checking the issue in building looks the same as Mapknitter-Stable

@icarito
Copy link
Member

icarito commented Jan 11, 2020

I was able to reproduce in the command line, looking for a fix

@icarito
Copy link
Member

icarito commented Jan 11, 2020

I figured it out, it was a permissions issue! Apparently somewhere in the process we create files by root? I've chowned them and it should work now - will keep an eye to see at what part of the process it happens again.

@icarito
Copy link
Member

icarito commented Jan 11, 2020

Okay so this branch is running in unstable staging branch. Stable is running also.
It calls my attention the presence of a core file. This usually indicates a crash somewhere, to pay attention.

@icarito
Copy link
Member

icarito commented Jan 11, 2020

I guess there are no debugging symbols in our passenger build but GDB reports this:

Core was generated by `Passenger AppPreloader: /app (forking...)                                     '.
Program terminated with signal SIGABRT, Aborted.
#0  0x00007fc4e85b3fff in ?? ()
(gdb) bt
#0  0x00007fc4e85b3fff in ?? ()
#1  0x0000000000000000 in ?? ()
(gdb)

@jywarren
Copy link
Member Author

jywarren commented Jan 11, 2020 via email

@sentry-io
Copy link

sentry-io bot commented Jan 11, 2020

Sentry issue: MAPKNITTER-M

@jywarren
Copy link
Member Author

Hm. I'm still getting this locally:

jquery.self-e200ee796ef24add7054280d843a80de75392557bb4248241e870fac0914c0c1.js?body=1:9838 POST http://export.mapknitter.org/export 500 (Internal Server Error)
send @ jquery.self-e200ee796ef24add7054280d843a80de75392557bb4248241e870fac0914c0c1.js?body=1:9838
ajax @ jquery.self-e200ee796ef24add7054280d843a80de75392557bb4248241e870fac0914c0c1.js?body=1:9435
_defaultFetchStatusUrl @ leaflet.distortableimage.self-3f907497d15bb5abe7bee4c432b0cee39ec3d28c608315257e7af8999b49adf1.js?body=1:2807
(anonymous) @ leaflet.distortableimage.self-3f907497d15bb5abe7bee4c432b0cee39ec3d28c608315257e7af8999b49adf1.js?body=1:2830
startExport @ leaflet.distortableimage.self-3f907497d15bb5abe7bee4c432b0cee39ec3d28c608315257e7af8999b49adf1.js?body=1:2761
addHooks @ leaflet.distortableimage.self-3f907497d15bb5abe7bee4c432b0cee39ec3d28c608315257e7af8999b49adf1.js?body=1:1790
enable @ leaflet.toolbar.self-226640c481c6ad02405acc7f132301ba4ced168b1f7e9db10f3c86cf9babf70f.js?body=1:1
enable @ leaflet.toolbar.self-226640c481c6ad02405acc7f132301ba4ced168b1f7e9db10f3c86cf9babf70f.js?body=1:1
handler @ LeafletEnvironmentalLayers.self-bfc185ab0b58c254f69ba81d8a71d041a396d8e4ba15a90522dc0e1046225c5b.js?body=1:14320

edit:1 Access to XMLHttpRequest at 'http://export.mapknitter.org/export' from origin 'http://localhost' has been blocked by CORS policy: No 'Access-Control-Allow-Origin' header is present on the requested resource.

Ah, i bet the 500 error is causing the CORS error, i think that happened before. Let me check, i bet the format is not quite right.

@jywarren
Copy link
Member Author

ah yes, it's because my images are local, so the exporter can't read them. I guess i have to test on unstable.

// exportUrl: 'http://34.74.118.242/api/v2/export/', // to swap to JS exporter
// exportStartUrl: 'http://34.74.118.242/api/v2/export/', // to swap to JS exporter
// fetchStatusUrl: fetchStatusUrl,
handleStatusResponse: handleStatusResponse
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not clear to me that this is getting used; need to console.log a bit.


// save the location of the status URL
$.ajax({
url: "/export",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for example this doesn't seem to be running...

@jywarren
Copy link
Member Author

Syncing this with publiclab/Leaflet.DistortableImage#519 too cc @sashadev-sky -- thanks!

@jywarren
Copy link
Member Author

If folks could try this out it'd be great cc @VladimirMikulic @cesswairimu

shift-select the images and press the export button in upper left of the map. Also, the Exports => Exports tab should show exports you've started from that page...

they won't properly run from local maps, so heads up.

Almost there! I'll try to pull in the latest LDI version too!

@VladimirMikulic
Copy link
Contributor

@jywarren I've started a few exports but they are not shown in the "Exports" tab. No console errors.

@jywarren
Copy link
Member Author

jywarren commented Jan 14, 2020 via email

@jywarren
Copy link
Member Author

I think we've got this, pretty much. I need help resolving the promise to cancel the export, though -- @sashadev-sky @VladimirMikulic do you have any tips for me here?

image

I'll try pushing this to unstable as soon as I can for testing!

@cesswairimu
Copy link
Collaborator

Awesome 🎉 🎉

// repeatedly fetch the status.json
var updateInterval = setInterval(function intervalUpdater() {
// need to bust cache with a rotating timestamp...
$.ajax('http://export.mapknitter.org/' + status_url + '?' + Date.now(), { // bust cache with timestamp
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: after we get the first status.json response, update the URL to go directly to Google Cloud, so we pay less

@VladimirMikulic
Copy link
Contributor

I think we've got this, pretty much. I need help resolving the promise to cancel the export, though -- @sashadev-sky @VladimirMikulic do you have any tips for me here?

You could wrap handleStatusResponse in a promise and then resolve out of it once the export is done/cancelled.

@jywarren
Copy link
Member Author

😕 -- oddly there is no tags.js that I can see... https://github.com/publiclab/mapknitter/tree/main/app/assets/javascripts/

But, are we able to compare to the manifest I referenced above where I did have it working locally? Or is that how you're comparing it?

Thanks!

@icarito
Copy link
Member

icarito commented Mar 24, 2020

Yes the comparison was from the manifests, where I found tags.js. If it's not familiar then it's likely it's an artifact of the build.

@icarito
Copy link
Member

icarito commented Mar 24, 2020

I found some missing commands when comparing the Makefile that we use in Plots2 vs this one, this looks to be the issue.

@icarito
Copy link
Member

icarito commented Mar 24, 2020

I feel I'm getting closer. I can reproduce the issue I think, that is, I seem to be unable to build assets in unstable, I get the following output instead of the expected:

jenkins@tycho:/srv/mapknitter_unstable/mapknitter$ docker-compose exec web rake assets:precompile
yarn install v1.22.4
[1/5] Validating package.json...
[2/5] Resolving packages...
success Already up-to-date.
Done in 0.34s.

@icarito
Copy link
Member

icarito commented Mar 24, 2020

Allright! I slowly reviewed all differences with plots2 and found .yarnrc which helps put assets in the right location for rails assets pipeline to find. So now it does build production assets!
Please review!

@jywarren
Copy link
Member Author

Wow, it seems to be working!!!

I think we can merge this now. There is some follow-up fixing we ought to do but it's good enough to publish... let me review the checklist first!

@jywarren
Copy link
Member Author

Yes, the next step is to merge this, the test on stable. 🙌

I'm waiting for my export to complete but otherwise all good!

OK the export worked well. But the alert didn't show. Will check this then merge.

@jywarren
Copy link
Member Author

Ah, i had removed the completion alert. Adding it back in then this is good to merge -- @icarito if you see it pass with my new commit, feel free to!

data = JSON.parse(data);
console.log(data, data.status, data.jpg);
if (data && data.status == "complete") {
alert("Export completed at " + data.cm_per_pixel + " cm/px. JPG available at https://mapknitter-exports-warps.storage.googleapis.com/" + data.jpg.split('public/warps')[1] + " -- Please refresh page to view completed exports.");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I set the alert to show the JPG and direct towards a reload. then cleared the interval. Wooo!

Copy link
Member

@icarito icarito left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks ok on unstable so far!

@icarito
Copy link
Member

icarito commented Mar 25, 2020

Ah, i had removed the completion alert. Adding it back in then this is good to merge -- @icarito if you see it pass with my new commit, feel free to!

Sure, except I don't seem to have the privileges to do so...

@jywarren
Copy link
Member Author

Hi @icarito i'm not sure the latest code is on unstable... it's not giving me an alert. Can you check to be sure the latest commits are up there? Thanks!

I'm testing at: https://unstable.mapknitter.org/maps/testmap/edit

@icarito
Copy link
Member

icarito commented Mar 25, 2020 via email

@jywarren
Copy link
Member Author

Ah, sorry!!! it's been a while since i did it!

@jywarren
Copy link
Member Author

Yay!!!

image

Merging!

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

Successfully merging this pull request may close these issues.

Re-connect saving status URL with Rails app
6 participants