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

feature: enhanced search capability #83

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

Conversation

aaronatbissell
Copy link
Contributor

Description

Type of change

This branch makes a couple of small tweaks to the enhanced search capability initially developed by @jonasneu-aws, specifically, the below updates:

  1. The migration instructions specify the parameters needed to deploy the “Neptune to Elasticsearch” solution. For the parameter “NeptuneClientSecurityGroup”, the current migration instructions say to use "The CDFSecurityGroupId output of the stack cdf-network-<cdf-environment-name>, but I had to use "The NeptuneSecurityGroupID output of the stack cdf-assetlibrary-neptune-<cdf-environment-name>”. The Neptune export Batch job was timing out after 30 seconds of trying to connect to Neptune unless I used this other security group.
  2. The new deployment adds a Neptune DB Cluster parameter group. In order for those parameters to take effect, all DB instances in the cluster need to be restarted (per the documentation here: https://docs.aws.amazon.com/neptune/latest/userguide/parameter-groups.html). This means the Stream Poller will not work correctly (it throws an error, saying Neptune Streams are disabled) until those DB instances are rebooted. We may want to add a step to the migration guidelines specifying that, before the user turns back on writes to the DB.
  3. The OpenSearch instance created with the new enhanced search cloudformation includes a security group with 2 ingress rules. A 3rd ingress rule was needed for that initial data load from Neptune to Elastic search (Step 3 from here).
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (existing code being refactored)
  • This change includes a documentation update

Submission Checklist

  • Build Verified
  • Bundle Verified
  • Lint passing
  • Unit tests passing
  • Integration tests passing
  • Change logs generated
Additional Notes:

@aaronatbissell
Copy link
Contributor Author

Any questions or requests I can help answer on this so we can get it merged in?

@aaronatbissell
Copy link
Contributor Author

This PR has been sitting for quite a while. Any chance this is even on the radar to get merged in?

@hassankhokhar @boardthatpowder @rrangnekar

@aaronatbissell
Copy link
Contributor Author

One last effort here to get any kind of feedback from anyone on the team, otherwise we will just fork this and manage it on our own. Is this even worth me going through and fixing all this stuff so that we can get this in?

@ts-amz
Copy link
Contributor

ts-amz commented Aug 16, 2023

@aaronatbissell, we are once again evaluating PRs to be merge. If you are able to fix the recent conflicts, we can review and merge this update.

@ts-amz
Copy link
Contributor

ts-amz commented Sep 7, 2023

Hello @aaronatbissell, are you planning to work on this PR? We plan to close this on 9/8/23 for tracking purposes if it's not still active (in that case, please re-open it if/when you have update it). Thank you

@aaronatbissell
Copy link
Contributor Author

I can also try to work on this one, but most of this PR was written by an AWS team member formerly on the CDF team. I made a few small tweaks at the end and submitted the PR. Won't be able to get to this for about a month, but I will try.

@aaronatbissell
Copy link
Contributor Author

@ts-amz - rebased on main. This will probably need some extensive testing

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