-
Notifications
You must be signed in to change notification settings - Fork 5
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
Remove blob #513
Remove blob #513
Conversation
terraform/ecs.tf
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Were these changes an intentional part of this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are also part of another PR #482 which was not approved/merged. SoI embedded it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Were these changes an intentional part of this PR?
log.debug("Excluded fields are " + excluded_fields); | ||
for (Map.Entry<String, Object> entry : sourceAsMap.entrySet()) { | ||
try { | ||
apiProperty = SearchUtil.openPropertyToJsonProperty(entry.getKey()); | ||
if ((excluded_fields == null) || !excluded_fields.contains(apiProperty)) | ||
log.debug("see if property " + apiProperty + "should be added"); | ||
if ((excluded_fields == null) || !excluded_fields.contains(apiProperty)) { | ||
log.debug("confirmed, the property is being added"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these lines intended to remain in the codebase, or were they temporary dev logs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right temporary logs. I can remove them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, with some questions (see comments)
🗒️ Summary
Removes the blob from the application/json and kvp responses.
⚙️ Test Data and/or Report
Test with postman, see PR : NASA-PDS/registry#301
♻️ Related Issues
Fixes #497
Fixes #514