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

[mds-geography][mds-geography-author] Moving read-only geo points out of geo author, leaving metadata endpo… #321

Merged
merged 1 commit into from
May 15, 2020

Conversation

janedotx
Copy link

@janedotx janedotx commented May 15, 2020

…ints.

PR Checklist

  • simple searchable title - [mds-db] Add PG env var, [config] Fix eslint config
  • briefly describe the changes in this PR
  • mark as draft if should not be merged
  • write tests for all new functionality

Impacts

  • Provider
  • Agency
  • Audit
  • Policy
  • Compliance
  • Daily
  • Native
  • Policy Author

These are breaking changes, but it doesn't make much sense to duplicate code between the two services. The specs (still in PR) have both been updated to reflect the new distribution of endpoints between the two services.

Copy link

@avatarneil avatarneil left a comment

Choose a reason for hiding this comment

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

Looks good.

if (err instanceof NotFoundError) {
return res.status(404).send({ error: err })
} catch (error) {
logger.error('failed to read geography', error.stack)

Choose a reason for hiding this comment

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

Can get rid of this logging line, we only care about logging the error in the case of a ServerError (which is handled by the global middleware now).

Copy link

@marie-x marie-x left a comment

Choose a reason for hiding this comment

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

Yay. Nice fixes to error chaining too.

@marie-x marie-x merged commit 3da8ad0 into develop May 15, 2020
@janedotx janedotx deleted the fix/align-geo-services-with-spec branch May 15, 2020 15:10
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