[FIX] Notebook AWS_REGION in .env and NextGen OpenSearch VPC endpoint - #516
[FIX] Notebook AWS_REGION in .env and NextGen OpenSearch VPC endpoint#516oussamahansal wants to merge 3 commits into
Conversation
mykola-pereyma
left a comment
There was a problem hiding this comment.
On the AWS_REGION change: every template already exports AWS_REGION into the notebook .env on main, so these additions are duplicates (one per template; existing-vpc ends up with three). Details inline. Harmless at runtime but redundant — suggest dropping the AWS_REGION additions unless there's a path where it's actually missing.
(Separately, the NextGen VPC-endpoint type swap is the substantive change and would benefit from confirmation it was deployed end-to-end — the AOSS network policy still references this endpoint in SourceVPCEs, and swapping the AOSS-managed endpoint for a generic EC2 interface endpoint needs to be verified against a live collection.)
mykola-pereyma
left a comment
There was a problem hiding this comment.
For the NextGen VPC-endpoint change (Change 2): it's a structurally sound CloudFormation swap, but its actual functioning depends on two AOSS control-plane behaviors that aren't verifiable from the template. Could you provide deploy confirmation? Specifics inline on the endpoint's ServiceName.
noel-improv
left a comment
There was a problem hiding this comment.
The change looks good, just one blocking item and suggestions referenced below
|
Lexical Graph Coverage Report: The coverage is at 65.82% (target: 80%). Download the HTML report here. |
noel-improv
left a comment
There was a problem hiding this comment.
lgtm, thanks for addressing the raised items
Description
Two fixes for the lexical-graph CloudFormation templates
Changes
Problem
Related issue (if any): #
Testing
pytest)Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.