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

Add Logging View #1080

Merged
merged 42 commits into from Jan 24, 2023
Merged

Add Logging View #1080

merged 42 commits into from Jan 24, 2023

Conversation

mfixstsci
Copy link
Collaborator

Adding page that will allow users to view logs interactively in the application.

@mfixstsci mfixstsci changed the title [WIP] Add Logging View Add Logging View Jan 23, 2023
@mfixstsci
Copy link
Collaborator Author

@BradleySappington or @bhilbert4 this is ready for a review when ready!

Copy link
Collaborator

@bhilbert4 bhilbert4 left a comment

Choose a reason for hiding this comment

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

Do you think we should be able to see logs from all servers regardless of what server the user is on?


full_log_paths = sorted(glob.glob(os.path.join(log_path, server, '*', '*')), reverse=True)
full_log_paths = [log for log in full_log_paths if not os.path.basename(log).startswith('.')]
log_dictionary = {os.path.basename(path): path for path in full_log_paths}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Dictionary comprehension! I had forgotten that was a thing. :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was super useful here.


full_uri = request.build_absolute_uri()

if 'dljwql' in full_uri:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you think this is something that would be useful to have as its own general function somewhere, or is there not enough use for it to warrant a separate function?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So this was causing the issue with the production logs being show on the test server. I went ahead and changed it to use the socket library to get the hostname of the machine rather than the URL.

Copy link
Collaborator

@BradleySappington BradleySappington left a comment

Choose a reason for hiding this comment

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

May be cleaner to keep javascript code in the javascript module when appropriate.

else:
server = 'ops'

full_log_paths = sorted(glob.glob(os.path.join(log_path, server, '*', '*')), reverse=True)
Copy link
Collaborator

Choose a reason for hiding this comment

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

you may need more tests to verify you are on the correct server. "pljwql" is showing up on the test server.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I went ahead and used a different library for this portion of the app. I have it working successfully now!

@mfixstsci mfixstsci merged commit 9b10807 into spacetelescope:develop Jan 24, 2023
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.

None yet

3 participants