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

initial changes to implement fog cloud storage library #822

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

icarito
Copy link
Member

@icarito icarito commented Dec 28, 2021

Fixes #821
This is an initial naive implementation modeled after publiclab/plots2#9807

  • tests pass -- rake test
  • code is in uniquely-named feature branch, and has been rebased on top of latest master (especially if you've been asked to make additional changes)
  • pull request are descriptively named
  • if possible, multiple commits squashed if they're smaller changes
  • reviewed/confirmed/tested by another contributor or maintainer

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

Thanks!

@gitpod-io
Copy link

gitpod-io bot commented Dec 28, 2021

@@ -53,6 +53,8 @@ gem 'tzinfo', '~> 1.2'
gem 'open_id_authentication'
gem 'protected_attributes_continued', '~> 1.8.2'
gem 'ruby-openid'
gem 'fog-google'
Copy link
Member

Choose a reason for hiding this comment

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

We could do this due to fog/fog-google#535

Suggested change
gem 'fog-google'
gem 'fog-google', '1.13.0' # Limited to `v1.13.0` due to https://github.com/fog/fog-google/issues/535

@@ -4,6 +4,17 @@
# Paperclip.options[:command_path] = "/usr/bin/identify"
# Paperclip.options[:command_path] = "/usr/local/bin"

Paperclip::Attachment.default_options[:storage] = :fog
Paperclip::Attachment.default_options[:fog_directory] = ENV["GOOGLE_STORAGE_BUCKET_NAME"] || ''
Paperclip::Attachment.default_options[:path] = ":rails_root/public/system/public/system/:class/:attachment/:id_partition/:style/:filename"
Copy link
Member

Choose a reason for hiding this comment

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

Sourcing from /app/models/spectrum.rb, this should be:

Suggested change
Paperclip::Attachment.default_options[:path] = ":rails_root/public/system/public/system/:class/:attachment/:id_partition/:style/:filename"
Paperclip::Attachment.default_options[:path] = ":rails_root/public/system/:attachment/:id/:style/:filename"

Or it may be id_partition instead of id. Let's try id first.

Copy link
Member

Choose a reason for hiding this comment

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

And we may need to again duplicate public/system to public/system/public/system so let's see there once this runs.

@jywarren
Copy link
Member

jywarren commented Jan 4, 2022

OK @icarito we need to test this out. There are two changes which may be necessary.

  • we may need to again duplicate public/system to public/system/public/system as we did in plots2, so let's see there once this runs.
  • we may need to change id to id_partition also. Let's see.

@@ -4,6 +4,17 @@
# Paperclip.options[:command_path] = "/usr/bin/identify"
# Paperclip.options[:command_path] = "/usr/local/bin"

Paperclip::Attachment.default_options[:storage] = :fog
Paperclip::Attachment.default_options[:fog_directory] = ENV["GOOGLE_STORAGE_BUCKET_NAME"] || ''
Paperclip::Attachment.default_options[:path] = ":rails_root/public/system/:attachment/:id/:style/:filename"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Paperclip::Attachment.default_options[:path] = ":rails_root/public/system/:attachment/:id/:style/:filename"
Paperclip::Attachment.default_options[:path] = ":rails_root/public/system/public/system/:attachment/:id/:style/:filename"

@jywarren
Copy link
Member

jywarren commented Jan 4, 2022

Confirming it works in GitPod. Now needs testing of in unstable of:

I'd like to pre-generate an "old" spectrum on unstable BEFORE rolling out this new code, so we can check the way old images are routed after this code is deployed. Currently https://unstable.spectralworkbench.org/ doesn't have any saved images, but i get an error when trying to save an image from https://unstable.spectralworkbench.org/capture

@icarito
Copy link
Member Author

icarito commented Jan 25, 2022

Okay I've created a bucket, called "spectralworkbench-bucket"

@icarito
Copy link
Member Author

icarito commented Jan 25, 2022

Will now add the GOOGLE_STORAGE_BUCKET_NAME var to unstable and push to unstable

@icarito
Copy link
Member Author

icarito commented Jan 25, 2022

I did implement the env vars in unstable and got:

I, [2022-01-25T19:31:43.766282 #1622]  INFO -- : [50960f42-092f-4d7e-a597-90cc0a25ca79] [paperclip] saving photos/1/original/porcupine.jpeg
D, [2022-01-25T19:31:45.802679 #1622] DEBUG -- : [50960f42-092f-4d7e-a597-90cc0a25ca79]    (60.2ms)  ROLLBACK
I, [2022-01-25T19:31:45.803583 #1622]  INFO -- : [50960f42-092f-4d7e-a597-90cc0a25ca79] Completed 500 Internal Server Error in 2683ms (ActiveRecord: 63.8ms)
F, [2022-01-25T19:31:45.804923 #1622] FATAL -- : [50960f42-092f-4d7e-a597-90cc0a25ca79]
F, [2022-01-25T19:31:45.804995 #1622] FATAL -- : [50960f42-092f-4d7e-a597-90cc0a25ca79] Errno::EISDIR (Is a directory @ io_fread - /app):
F, [2022-01-25T19:31:45.805046 #1622] FATAL -- : [50960f42-092f-4d7e-a597-90cc0a25ca79]
F, [2022-01-25T19:31:45.805136 #1622] FATAL -- : [50960f42-092f-4d7e-a597-90cc0a25ca79] app/controllers/spectrums_controller.rb:230:in `create'
I, [2022-01-25T19:31:46.104982 #1622]  INFO -- : [99ce8e1d-5707-4597-a193-c263310a5d78] Started GET "/assets/application.css" for 186.102.12.238 at 2022-01-25 19:31:46 +0000
I, [2022-01-25T19:31:46.147964 #1641]  INFO -- : [051b484c-ab41-42f6-8e3c-ba681a45a89b] Started GET "/assets/application.js" for 186.102.12.238 at 2022-01-25 19:31:46 +0000

when trying to upload!

@icarito
Copy link
Member Author

icarito commented Jan 25, 2022

Perhaps it's an authentication issue...

@jywarren
Copy link
Member

Is there any more trace, deeper into the stack? The line number cited is just for spectrum.save:

@icarito
Copy link
Member Author

icarito commented Jan 25, 2022

Yes it would seem it was an authentication issue. I've added a serviceaccount and now I'm seeing permissions errors.

@icarito
Copy link
Member Author

icarito commented Jan 25, 2022

I'm granting permission to the service account and will try again.

@icarito
Copy link
Member Author

icarito commented Jan 25, 2022

The error says:

F, [2022-01-25T20:38:40.864707 #1629] FATAL -- : [99a844b5-7565-42cc-8785-418685dbd702] Google::Apis::ClientError (forbidden: specwb-files@public-lab.iam.gserviceaccount.com does not have storage.objects.create access to the Google Cloud Storage object.):

Perhaps I've misconfigured the service account, I'll check it!

@icarito
Copy link
Member Author

icarito commented Jan 25, 2022

After granting the appropriate permissions to the service account, I've confirmed unstable is capable of uploading files to a cloud bucket. 🎆 So we should merge this!

@icarito
Copy link
Member Author

icarito commented Jan 25, 2022

Actually there's a new error now!

I, [2022-01-25T21:38:32.764925 #1629]  INFO -- : [2076a135-fd67-41b7-8829-023859b66a6d] Completed 500 Internal Server Error in 1699ms (ActiveRecord: 47.8ms)
F, [2022-01-25T21:38:32.767038 #1629] FATAL -- : [2076a135-fd67-41b7-8829-023859b66a6d]
F, [2022-01-25T21:38:32.767150 #1629] FATAL -- : [2076a135-fd67-41b7-8829-023859b66a6d] Magick::ImageMagickError (unable to open image `public/photos/4/original/640x800_1d6f3dc8-8cf3-4d9b-9d41-c80df8bf9d86.jpg': No such file or directory @ error/blob.c/OpenBlob/2701):
F, [2022-01-25T21:38:32.767248 #1629] FATAL -- : [2076a135-fd67-41b7-8829-023859b66a6d]
F, [2022-01-25T21:38:32.767408 #1629] FATAL -- : [2076a135-fd67-41b7-8829-023859b66a6d] app/models/spectrum.rb:281:in `new'
[2076a135-fd67-41b7-8829-023859b66a6d] app/models/spectrum.rb:281:in `rotate'
[2076a135-fd67-41b7-8829-023859b66a6d] app/controllers/spectrums_controller.rb:242:in `block in create'
[2076a135-fd67-41b7-8829-023859b66a6d] app/controllers/spectrums_controller.rb:232:in `create'
I, [2022-01-25T21:38:32.982645 #1629]  INFO -- : [0bc49a2c-c835-4403-b430-9d01af1b19a4] Started GET "/assets/application.css" for 186.102.12.238 at 2022-01-25 21:38:32 +0000
I, [2022-01-25T21:38:32.982999 #1648]  INFO -- : [a2065b62-c967-4b2e-8719-0df19dc0c018] Started GET "/assets/application.js" for 186.102.12.238 at 2022-01-25 21:38:32 +0000

I wonder if this is a more complex issue, such as wanting to be processing the image in the bucket.

@icarito
Copy link
Member Author

icarito commented Jan 25, 2022

We're getting closer, I think, now there's a different error:

I, [2022-01-25T22:26:49.552638 #1627]  INFO -- : [c52d0fa7-1e5c-4436-8ee8-dbbd7627d81c] Completed 500 Internal Server Error in 1559ms (ActiveRecord: 39.5ms)
F, [2022-01-25T22:26:49.554012 #1627] FATAL -- : [c52d0fa7-1e5c-4436-8ee8-dbbd7627d81c]
F, [2022-01-25T22:26:49.554099 #1627] FATAL -- : [c52d0fa7-1e5c-4436-8ee8-dbbd7627d81c] ArgumentError (wrong number of arguments (given 1, expected 2)):
F, [2022-01-25T22:26:49.554194 #1627] FATAL -- : [c52d0fa7-1e5c-4436-8ee8-dbbd7627d81c]
F, [2022-01-25T22:26:49.554306 #1627] FATAL -- : [c52d0fa7-1e5c-4436-8ee8-dbbd7627d81c] app/models/spectrum.rb:283:in `rotate'
[c52d0fa7-1e5c-4436-8ee8-dbbd7627d81c] app/controllers/spectrums_controller.rb:242:in `block in create'
[c52d0fa7-1e5c-4436-8ee8-dbbd7627d81c] app/controllers/spectrums_controller.rb:232:in `create'
I, [2022-01-25T22:26:49.755078 #1627]  INFO -- : [484833e3-d1f6-4f4e-a83e-fa63fb8dd236] Started GET "/assets/application.js" for 186.102.12.238 at 2022-01-25 22:26:49 +0000
I, [2022-01-25T22:26:50.048229 #1627]  INFO -- : [f086af1c-2536-4e02-a284-1989121dea6b] Started GET "/assets/application.css" for 186.102.12.238 at 2022-01-25 22:26:50 +0000

@@ -114,7 +114,8 @@ def self.weekly_tallies

# finds the brightest row of the image and uses that as its sample row
def find_brightest_row
image = Magick::ImageList.new('public' + (photo.url.split('?')[0]).gsub('%20', ' '))
photo.copy_to_local_file(:original,local_photo_path)
Copy link
Member

Choose a reason for hiding this comment

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

@jywarren
Copy link
Member

OK:

Error:
SpectrumsControllerTest#test_should_calibrate_spectrum:
Magick::ImageMagickError: improper image header `public/photos/980190962/original/capture.png' @ error/png.c/ReadPNGImage/4092

um, how are we doing it wrong?

@icarito
Copy link
Member Author

icarito commented Feb 8, 2022

Now seeing:

I, [2022-02-08T21:52:26.685846 #1619]  INFO -- : [d7c581c3-4158-41ea-93a2-2bb98ede476a] Completed 500 Internal Server Error in 1835ms (ActiveRecord: 78.8ms)
F, [2022-02-08T21:52:26.687723 #1619] FATAL -- : [d7c581c3-4158-41ea-93a2-2bb98ede476a]
F, [2022-02-08T21:52:26.687855 #1619] FATAL -- : [d7c581c3-4158-41ea-93a2-2bb98ede476a] Magick::ImageMagickError (WriteBlob Failed `public/photos/3/original/avatar.png' @ error/png.c/MagickPNGErrorHandler/1628):
F, [2022-02-08T21:52:26.687977 #1619] FATAL -- : [d7c581c3-4158-41ea-93a2-2bb98ede476a]
F, [2022-02-08T21:52:26.688148 #1619] FATAL -- : [d7c581c3-4158-41ea-93a2-2bb98ede476a] app/models/spectrum.rb:286:in `write'
[d7c581c3-4158-41ea-93a2-2bb98ede476a] app/models/spectrum.rb:286:in `rotate'
[d7c581c3-4158-41ea-93a2-2bb98ede476a] app/controllers/spectrums_controller.rb:242:in `block in create'
[d7c581c3-4158-41ea-93a2-2bb98ede476a] app/controllers/spectrums_controller.rb:232:in `create'

@icarito
Copy link
Member Author

icarito commented Feb 8, 2022

Tried with a JPEG:

I, [2022-02-08T21:55:35.975286 #1619]  INFO -- : [ae7d1f7a-afb8-4e2c-9d75-20c3193aa74c] Completed 500 Internal Server Error in 1960ms (ActiveRecord: 76.3ms)
F, [2022-02-08T21:55:35.976276 #1619] FATAL -- : [ae7d1f7a-afb8-4e2c-9d75-20c3193aa74c]
F, [2022-02-08T21:55:35.976339 #1619] FATAL -- : [ae7d1f7a-afb8-4e2c-9d75-20c3193aa74c] Magick::ImageMagickError (unable to open image `public/photos/4/original/porcupine.jpeg': No such file or directory @ error/blob.c/OpenBlob/2701):
F, [2022-02-08T21:55:35.976476 #1619] FATAL -- : [ae7d1f7a-afb8-4e2c-9d75-20c3193aa74c]
F, [2022-02-08T21:55:35.976554 #1619] FATAL -- : [ae7d1f7a-afb8-4e2c-9d75-20c3193aa74c] app/models/spectrum.rb:286:in `write'
[ae7d1f7a-afb8-4e2c-9d75-20c3193aa74c] app/models/spectrum.rb:286:in `rotate'
[ae7d1f7a-afb8-4e2c-9d75-20c3193aa74c] app/controllers/spectrums_controller.rb:242:in `block in create'
[ae7d1f7a-afb8-4e2c-9d75-20c3193aa74c] app/controllers/spectrums_controller.rb:232:in `create'

@jywarren
Copy link
Member

jywarren commented Feb 8, 2022

  • pre-generate an "old" spectrum on unstable BEFORE FOG code
  • push out new FOG code
  • check that previous spectra images are still visible with local storage set in FOG via env variable
  • upload tycho test images from unstable to Google Cloud
  • reset for Google Cloud storage and check that old images still work
  • check that new images upload correctly with FOG code and Google Storage
  • put all env variables in correct places
  • if all above work, it's not a breaking change, and we can merge this PR.

To deploy this to production and enable it, we'd:

  • SQL database setup for SWB
  • copy all old SWB (non-test -- production) images to Google Cloud
  • switch to GC-backed Fog storage in env variable on production
  • publish code to production

@icarito
Copy link
Member Author

icarito commented Feb 15, 2022

Added a patch that fixes #891 to continue with our checklist

@icarito
Copy link
Member Author

icarito commented Feb 15, 2022

There is one big difference, with Fog_provider=Local images upload to public/photos but with previous main branch, images upload to system/public/photos

@jywarren
Copy link
Member

Just noting that we seem to be OK here, just waiting to be tested, (excepting the conflicts). 👍

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

Successfully merging this pull request may close these issues.

Please implement Google Cloud Storage for SW
2 participants