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

MIDRC-921: disable "download manifest" for more terms than the ES limit #1670

Merged
merged 3 commits into from
Feb 11, 2025

Conversation

paulineribeyre
Copy link
Contributor

Link to JIRA ticket if there is one: https://ctds-planx.atlassian.net/browse/MIDRC-921

New Features

Breaking Changes

Bug Fixes

Improvements

  • Explorer page: Do not allow users to click "download manifest" button when the query to ElasticSearch would fail due to having too many terms in the query. Instead, let the user know: "too many rows to download at once; make a smaller selection"

Dependency updates

Deployment changes

@paulineribeyre paulineribeyre changed the title MIDRC-921: disable "download manifest" for more than the max ES terms MIDRC-921: disable "download manifest" for more terms than the ES limit Feb 10, 2025
ocshawn
ocshawn previously approved these changes Feb 10, 2025
Copy link
Contributor

@ocshawn ocshawn left a comment

Choose a reason for hiding this comment

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

looks good

Copy link
Contributor

@frankliuao frankliuao left a comment

Choose a reason for hiding this comment

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

This will resolve the issue of forever-bouncing ••• in the portal, that MIDRC is facing now.

Copy link

filepath $$\textcolor{#23d18b}{\tt{passed}}$$ $$\textcolor{#f14c4c}{\tt{failed}}$$ $$\textcolor{#ffa500}{\tt{skipped}}$$ SUBTOTAL
$$\textcolor{#f14c4c}{\tt{tests/test\_ras\_authn.py}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#f14c4c}{\tt{2}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#f14c4c}{\tt{3}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_requestor.py}}$$ $$\textcolor{#23d18b}{\tt{5}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#23d18b}{\tt{5}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_data\_upload.py}}$$ $$\textcolor{#23d18b}{\tt{2}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#23d18b}{\tt{2}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_indexing\_page.py}}$$ $$\textcolor{#23d18b}{\tt{2}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#23d18b}{\tt{2}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_audit\_service.py}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_login\_page.py}}$$ $$\textcolor{#23d18b}{\tt{2}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#23d18b}{\tt{2}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_homepage.py}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_pfb\_export.py}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_discoverypage.py}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_graph\_submit\_and\_query.py}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#ffa500}{\tt{tests/test\_gen3ff\_landing\_page.py}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#ffa500}{\tt{2}}$$ $$\textcolor{#ffa500}{\tt{2}}$$
$$\textcolor{#ffa500}{\tt{tests/test\_register\_user.py}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#ffa500}{\tt{2}}$$ $$\textcolor{#ffa500}{\tt{2}}$$
$$\textcolor{#f14c4c}{\tt{TOTAL}}$$ $$\textcolor{#23d18b}{\tt{17}}$$ $$\textcolor{#f14c4c}{\tt{2}}$$ $$\textcolor{#ffa500}{\tt{4}}$$ $$\textcolor{#f14c4c}{\tt{23}}$$

Please find the detailed integration test report here

Please find the ci env pod logs here

Copy link

filepath $$\textcolor{#23d18b}{\tt{passed}}$$ $$\textcolor{#f14c4c}{\tt{failed}}$$ SUBTOTAL
$$\textcolor{#f14c4c}{\tt{tests/test\_ras\_authn.py}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#f14c4c}{\tt{2}}$$ $$\textcolor{#f14c4c}{\tt{3}}$$
$$\textcolor{#f14c4c}{\tt{TOTAL}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#f14c4c}{\tt{2}}$$ $$\textcolor{#f14c4c}{\tt{3}}$$

Please find the detailed integration test report here

Please find the ci env pod logs here

@paulineribeyre
Copy link
Contributor Author

Ignoring known issue causing test failure

@paulineribeyre paulineribeyre merged commit e841054 into master Feb 11, 2025
8 of 9 checks passed
@paulineribeyre paulineribeyre deleted the fix/file-manifest-button branch February 11, 2025 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants