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

CUMULUS-3847: Remove any remaining ES indexing functions and tests #3897

Open
wants to merge 7 commits into
base: feature/es-phase-2
Choose a base branch
from

Conversation

charleshuang80
Copy link
Contributor

…loud Metrics usage

Summary: Summary of changes

Addresses CUMULUS-3847: Remove any remaining ES indexing functions and tests

Changes

  • Detailed list or prose of changes
  • ...

PR Checklist

  • Update CHANGELOG
  • Unit tests
  • Ad-hoc testing - Deploy changes and test manually
  • Integration tests

});

test.serial('writePdr() saves a PDR record to PostgreSQL/Elasticsearch if PostgreSQL write is enabled', async (t) => {
test.serial('writePdr() saves a PDR record to PostgreSQL if PostgreSQL write is enabled', async (t) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can remove if PostgreSQL write is enabled

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can rename this file with different extension since we still want cleanExecution functionality, and will be updated in a different ticket.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest to comment out the code, and put a dummy handler, so the lambda can still be created in tf-modules/archive/clean_executions.tf

@@ -4,6 +4,7 @@
* It helps:
* - adding ElasticSearch index mapping when a new index is created
*/
// is this needed for CloudMetrics?
Copy link
Contributor

Choose a reason for hiding this comment

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

With es instance going away, this lambda shouldn't be needed.

@@ -188,6 +188,7 @@ test('ec2() service defaults to localstack in test mode', async (t) => {
);
});

// can this test be removed or is it needed for the granule CloudMetrics queries?
Copy link
Contributor

Choose a reason for hiding this comment

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

With es instance going away, I think packages/aws-client/tests/services.js es and this test are not needed.

@@ -969,6 +954,7 @@ async function handler(event) {
process.env.CMR_PAGE_SIZE = process.env.CMR_PAGE_SIZE || '200';

//TODO: Remove irrelevant env vars from terraform after ES reports are removed
// can this be changed now?
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, we are not querying es anymore.

@@ -49,7 +49,7 @@ const esSearchStub = sandbox.stub();
const esScrollStub = sandbox.stub();
FakeEsClient.prototype.scroll = esScrollStub;
FakeEsClient.prototype.search = esSearchStub;

// is this es-client stuff still needed b/c granules and metrics?
Copy link
Contributor

Choose a reason for hiding this comment

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

I think es-client is still needed. You can try to take it out.

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.

2 participants