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

Update main.py - MIME Type & Metadata #13279

Closed
wants to merge 2 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion functions/imagemagick/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def __blur_image(current_blob):

# Blur the image using ImageMagick.
with Image(filename=temp_local_filename) as image:
image.resize(*image.size, blur=16, filter="hamming")
image.resize(*image.size, blur=160, filter="hamming")
Copy link
Contributor

Choose a reason for hiding this comment

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

You are correct here that the blur value doesn't work. There's a different change that's required here to correct this, as it looks like the underlying library (Wand) has changed since this sample was implemented.

Context: other samples use 0x8 for the value, which in the CLI maps to a radius and sigma value, which was first implemented in Wand 0.4.5. This sample was written originally using 0.4.4

Copy link
Contributor

Choose a reason for hiding this comment

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

Using the current version of Wand, this is the equivalent implementation to a -blur 0x8 from other samples that use the command line:

Suggested change
image.resize(*image.size, blur=160, filter="hamming")
image.blur(radius=0, sigma=16)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The blur does work, but it is not strong enough to hide the underlying picture.

Copy link
Contributor

Choose a reason for hiding this comment

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

My suggested updates the blur method to match the other implementations of this sample that call the CLI directly.

image.save(filename=temp_local_filename)

print(f"Image {file_name} was blurred.")
Expand All @@ -81,6 +81,19 @@ def __blur_image(current_blob):
blur_bucket_name = os.getenv("BLURRED_BUCKET_NAME")
blur_bucket = storage_client.bucket(blur_bucket_name)
new_blob = blur_bucket.blob(file_name)

# Copy mimetype from the source blob.
new_blob.content_type = current_blob.content_type

# Add custom metadata.
# Add custom metadata.
Copy link
Contributor

Choose a reason for hiding this comment

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

Duplicate comment?

import datetime
new_blob.metadata = {
"blurred": "true",
"original_file": file_name,
"blurred_at": datetime.datetime.utcnow().isoformat(),
}
Comment on lines +84 to +95
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Copy mimetype from the source blob.
new_blob.content_type = current_blob.content_type
# Add custom metadata.
# Add custom metadata.
import datetime
new_blob.metadata = {
"blurred": "true",
"original_file": file_name,
"blurred_at": datetime.datetime.utcnow().isoformat(),
}

If you would like a version of your PR to be accepted (as you did correctly identify the blur issue and should get credit in resolving it), this additional functionality will have to be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll just use my fork.


Comment on lines +90 to +96
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason why you're adding this functionality?

For context: This is one of multiple implementations of the same functionality across multiple programming languages. We try and keep all implementations functionality the same, so adding this needs to be considered in the context of other samples.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. When I teach Data Engineering: I use this as an event driven processing example, and using the metadata to store the data load stage and discuss that the metadata is held in Spanner
  2. When I teach Application Development: I use metadata as an example to define when images are safe to display in the app

If this was production the code would need to always create the file in the target bucket and optionally blur it based on the ML result.

Copy link
Contributor

Choose a reason for hiding this comment

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

This sample is designed to demonstrate Cloud Run functions, the Cloud Vision API, and ImageMagick to detect and blur offensive images that get uploaded to a Cloud Storage bucket. It is not designed for your classroom. You are free to reuse the code for your own requirements as long as you adhere to the open source license attached.

new_blob.upload_from_filename(temp_local_filename)
print(f"Blurred image uploaded to: gs://{blur_bucket_name}/{file_name}")

Expand Down