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

Added ability to deny unfiltered historical queries if new config opt… #176

Merged

Conversation

chesterhawkins
Copy link
Contributor

closes #175

New config option sessions.historicalSessionFilter. denyUnfilteredQueries is used to determine whether an unfiltered historical query should be denied or allowed.

…ion "denyUnfilteredQueries" is defined or true.

New option is a sub-option of sessions.historicalSessionFilter
@jvigliotta jvigliotta self-requested a review July 25, 2024 23:32
Copy link
Collaborator

@jvigliotta jvigliotta left a comment

Choose a reason for hiding this comment

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

This looks great! Can we change the double quotes to single quotes for the message (we're trying to stick to this moving forward)? Also, can we update the config to add the new option, but default it to false.

Copy link
Collaborator

@davetsay davetsay left a comment

Choose a reason for hiding this comment

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

Hi @chesterhawkins . thanks so much for contributing. I have one more slight change. I added a code suggestion to also check for the disable prop not being true. If it is false we shouldn't care about the denyUnfilteredQueries prop.

src/historical/HistoricalProvider.js Outdated Show resolved Hide resolved
Copy link

sonarcloud bot commented Aug 13, 2024

Copy link
Collaborator

@davetsay davetsay left a comment

Choose a reason for hiding this comment

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

Awesome. Thanks, @chesterhawkins

@jvigliotta jvigliotta self-requested a review August 14, 2024 01:14
Copy link
Collaborator

@jvigliotta jvigliotta left a comment

Choose a reason for hiding this comment

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

LGTM!

@davetsay davetsay merged commit 872d521 into NASA-AMMOS:main Aug 14, 2024
3 checks passed
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 config option to disallow unfiltered historical queries
3 participants