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

Cv2 5086 smooch nlu to presto #2010

Conversation

DGaffney
Copy link
Contributor

Description

More work to move some of the lower level Alegre uses over to presto.

References: CV2-5086

How has this been tested?

Not yet tested - changing code first to determine effect of changes then work through necessary changes from there - testing will need to just ensure that articles do in fact get indexed if you send these commands to alegre from check-api

Things to pay attention to during code review

Nothing in particular

Checklist

  • I have performed a self-review of my own code
  • I have added unit and feature tests, if the PR implements a new feature or otherwise would benefit from additional testing
  • I have added regression tests, if the PR fixes a bug
  • I have added logging, exception reporting, and custom tracing with any additional information required for debugging
  • I considered secure coding practices when writing this code. Any security concerns are noted above.
  • I have commented my code in hard-to-understand areas, if any
  • I have made needed changes to the README
  • My changes generate no new warnings
  • If I added a third party module, I included a rationale for doing so and followed our current guidelines

DGaffney and others added 7 commits August 27, 2024 05:58
* CV2-5087 move Articles side effecting saves to to it via presto

* CV2-5082 move article indexing to presto

* resolve test errors

* updates for broken tests

* small tweak

* set to sync

* more fixes

* rename function and revert request

* add response suppression and move to specific path for side effecting requests

* extend similar media to allow for temporary texts

* fix broken test fixture

* revert back to async

* fix another test

* fixes per PR review

* fixes per PR review

* more fixes after review
Allow lists (core, custom, search results, articles, filtered or not) to be exported to a CSV file that is sent to e-mail.

- [x] Add an `exportList` GraphQL mutation
- [x] Implement a generic export class that supports media, articles and feeds
- [x] Validate maximum number of results (which is a global configuration key)
- [x] Validate permission
- [x] Create Sidekiq job to export results
- [x] Create a CSV for the export
- [x] Save CSV in S3 using a pre-signed URL that expires after X days ("X" is a global configuration key)
- [x] Add support to MailCatcher
- [x] Send CSV by e-mail
- [x] Automated tests
- [x] Make sure it works for articles as well
- [x] Make sure it works for shared feeds as well

References: CV2-5067 and CV2-4979.
@DGaffney DGaffney changed the base branch from develop to epic/cv2-5050-text-vectorization-via-presto August 27, 2024 19:54
@DGaffney DGaffney force-pushed the epic/cv2-5050-text-vectorization-via-presto branch from cbffa69 to ff95711 Compare August 29, 2024 14:05
@DGaffney DGaffney closed this Sep 5, 2024
@DGaffney DGaffney deleted the cv2-5086-smooch-nlu-to-presto branch September 5, 2024 12:09
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.

3 participants