-
Notifications
You must be signed in to change notification settings - Fork 0
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
HackParser integration #413
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kylehoehns
reviewed
Oct 10, 2024
kylehoehns
approved these changes
Oct 10, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
jsmilliken
temporarily deployed
to
Development
October 15, 2024 15:06 — with
GitHub Actions
Inactive
jsmilliken
had a problem deploying
to
Production
October 15, 2024 15:13 — with
GitHub Actions
Failure
Temporarily disable test coverage threshold
Re-enable testing threshold Remove uncessary node-fetch
Add logging of errors to console.
Add error handling to HackParser integration Fix edge case around code results block being added when there are just plain results Add remaining tests Revert getReviewByThreadId addition
Improve cleanup of environment variable changes during testing
Normalize style of requestReview test to reduce uncessary duplicate executions Add assertion helper to acceptRequestReview Remove duplicagte test from acceptRequestReview
jsmilliken
force-pushed
the
feature/s3-signed-clean-code-urls
branch
from
October 18, 2024 08:33
d8241ef
to
e27b100
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Integrate with the HackParser service.
The three functional changes are:
Each of these functional points have been made fail-proof, so if the HackParser service causes any errors, they will be logged, but not result in a crash of Hacker Rank Queue.
The actual HackParser integration is controlled by via the
HACK_PARSER_BUCKET_NAME
, when provided it's the name of the bucket. When not provided, the integration is fully disabled - not even allowing users to upload PDFs, and fully disabling the relevant code branches.This PR enables the HackParser integration in dev only - there will be another PR in the future to enable it in prod when HackParser is ready.
Aside from the integration changes, the style of
requestReview.test.ts
was changed to call the callback for each test within the test block compared to the previous approach of within thebeforeEach
.This change was made as the input to the callback was no longer the same for every test - in order to test if a review does not upload a PDF, etc - resulting in some new tests having to call the callback twice and make abnormal asserts.