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

FI-3263: Huge Bundle Fix #39

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

Conversation

emichaud998
Copy link
Collaborator

@emichaud998 emichaud998 commented Dec 4, 2024

Summary

This PR updates the way Service Base URL Bundles are validated so that the Bundle validation test will not fail if the Bundle is extremely large. Rather that sending the full bundle to the validator, the Bundle validation test will test for each of the Bundle constraints and required fields manually. The validator has also been updated to limit the number of validation messages/errors to 50 so that the UI does not get bogged down with validation messages. The Limit Validation to a Maximum Resource Count input has been added to give the user the ability to choose how many Bundle resources they want validated. If the user submits that they want X resources validated, Inferno will limit the number of Bundle entries down to around X resources by looping through each Organization resource and adding any referenced endpoints, or parent Organization and it's referenced endpoints. This substantially cuts down the time it takes these tests to run for large Bundles. However, large Bundles can now be ran without Inferno crashing, though it may slow the system down and take a while to complete, around 20-30 minutes depending on the size of the Bundle.

Testing Guidance

Ensure running our tests against https://brand-browser.argo.run/bundle.json no longer results in a validator 400 error. Then test out running the tests against https://brand-browser.argo.run/bundle.json with the Limit Validation to a Maximum Resource Count input populated to view how it cuts down the run time of the tests.

…dd input to

limit entries that are validated
…_valid_bundle_entries, and add filter to limit the number of warning/error messages that appear from the validator
@emichaud998 emichaud998 self-assigned this Dec 4, 2024
@arscan
Copy link
Contributor

arscan commented Dec 9, 2024

@emichaud998 please remove the .gem file, that shouldn't get committed.


add_message('error', %(
Organization with id: #{organization.id} references an Endpoint that is not contained in this
bundle."
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
bundle."
bundle.

Don't need trailing quote.


@messages = []
@messages += error_messages.first(20) unless error_messages.empty?
@messages += non_error_messages.first(50) unless non_error_messages.empty?
Copy link
Contributor

Choose a reason for hiding this comment

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

@Jammjammjamm -- is this an acceptable method to manually prune error messages coming from the validator so it doesn't crash the interface? I seem to recall the validator currently automatically adds messages to the built-in-message system. This seems risky but i'm not sure we have any other better approach, unless you know of one?

Copy link
Contributor

Choose a reason for hiding this comment

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

The validator adds the messages by sticking them in @messages. Those messages are then persisted by the test runner after the test finishes executing, so that part seems fine.

I don't know why we'd want to allow more info/warning messages than error messages, though, as error messages are more important. It's probably also a good idea to put the message saying that only the first xyz messages are being shown first rather than last.

I also think a better solution would be to check if the limit has been reached after each iteration, and then breaking once the limit has been reached. That would prevent performing useless validations and also ensure that all of the messages for each resource validated are kept.

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 was also thinking that would be a better solution, but the issue with that approach is if there are a ton of info/warning messages. The problem I realized with that approach is that if there are a ton of info/warning messages then you might hit the limit of messages and break before you validate a resource that would have had an error and you wouldn't see it, whereas with this approach if there is an error, it will be caught.

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.

4 participants