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

Issue 167 HTML file Support #171

Merged
merged 5 commits into from
Nov 19, 2024
Merged

Issue 167 HTML file Support #171

merged 5 commits into from
Nov 19, 2024

Conversation

Hoshi-Okami
Copy link
Collaborator

@Hoshi-Okami Hoshi-Okami commented Nov 11, 2024

Fixes #167

Modified the process_gcs_directory function to detect HTML files by generating a public URL for each HTML file. In addition, I ensured that the JSON includes a path attribute for HTML files containing the public URL.

The GCS bucket in the process_gcs_directory did not include HTML files because it didn't recognize them. Including HTML files that are categorized properly helps reduce integration issues later and allows for easier testing and validation.

Checked the JSON response for HTML file details with a valid path and now HTML reports can be found under the acceptance reports.

@Hoshi-Okami Hoshi-Okami linked an issue Nov 11, 2024 that may be closed by this pull request
3 tasks
@Hoshi-Okami Hoshi-Okami changed the title html file links Issue 167 HTML file Support Nov 11, 2024
Copy link
Collaborator

@nguzinski nguzinski left a comment

Choose a reason for hiding this comment

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

some code repetition and .vscode being uploaded instead of ignored, otherwise it looks good

@@ -0,0 +1,3 @@
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wanna say that .vscode should be in the .gitignore, currently, it just ignores .vs, if you're able, add .vscode to the .gitignore

Copy link
Member

Choose a reason for hiding this comment

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

As Noah said, this should be added to .gitignore.

if blob.name.endswith('.txt'):
# Download the text content
file_contents = blob.download_as_text()

Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe change this to case switch/ pattern matching to make more readable

@@ -175,7 +212,6 @@ def process_gcs_directory(prefix, result, fuzzy_path_value):
"failContent": fail_content
}

# Determine the monitor type and append the file data
if "UnorderedWaypointMonitor" in blob.name:
Copy link
Collaborator

Choose a reason for hiding this comment

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

seems to be a repeat of above, maybe a way to make reduce code repetition


elif blob.name.endswith('.txt'):
file_contents = blob.download_as_text()

info_content = get_info_contents(file_contents, "INFO", {})
pass_content = get_info_contents(file_contents, "PASS", {})
fail_content = get_info_contents(file_contents, "FAIL", {})

file_data = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

repeat of above, some way to reduce code repition

Copy link
Member

@mohamdlog mohamdlog left a comment

Choose a reason for hiding this comment

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

Could you help me understand why I'm getting data:image/png;base64, undefined from your implementation? Here’s what I’m seeing before and after using it:

Screenshot 2024-11-12 121602
Screenshot 2024-11-12 121655

Nice work overall. Noah brings up good points, please be sure to address those.

@@ -0,0 +1,3 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

As Noah said, this should be added to .gitignore.

Copy link
Member

@mohamdlog mohamdlog left a comment

Choose a reason for hiding this comment

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

Your code is not fetching HTML files, tested with print(json.dumps(result, indent=2)). your implementation is also creating a problem with the PNGs:
Screenshot 2024-11-19 150003

Copy link
Member

@mohamdlog mohamdlog left a comment

Choose a reason for hiding this comment

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

Great work. Here's a summary of my changes: added a proxy endpoint to securely serve HTML files, updated the folder contents API to include links to these HTML files, and ensured each file is correctly referenced for frontend use.

Screenshot 2024-11-19 155759

@mohamdlog mohamdlog merged commit b65ae54 into main Nov 19, 2024
2 checks passed
@mohamdlog mohamdlog deleted the issue167 branch November 29, 2024 08:28
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.

Add Backend Support for HTML File Links
3 participants