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

[#175496403] Removed authentication and standard endpoint for health check #100

Merged
merged 2 commits into from
Nov 5, 2020

Conversation

pasqualedevita
Copy link
Member

No description provided.

@pagopa-github-bot
Copy link
Contributor

pagopa-github-bot commented Oct 29, 2020

Warnings
⚠️

Please include a description of your PR changes.

Affected stories

  • 🌟 #175496403: Standardizzare e rimuovere autenticazione per endpoint health check functions

Generated by 🚫 dangerJS

@codecov-io
Copy link

codecov-io commented Oct 29, 2020

Codecov Report

Merging #100 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #100   +/-   ##
=======================================
  Coverage   83.06%   83.06%           
=======================================
  Files          49       49           
  Lines        1671     1671           
  Branches      126      126           
=======================================
  Hits         1388     1388           
  Misses        278      278           
  Partials        5        5           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 13b8375...3bafb4d. Read the comment docs.

"type": "httpTrigger",
"direction": "in",
"name": "req",
"route": "adm/info",
"route": "info",
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need this or is just a matter of style?

Copy link
Contributor

Choose a reason for hiding this comment

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

we need this as when the API is reachable through the API gateway we need a prefix to discriminate the target functions (backend)

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that is a good practice have the same standard API for all our health checks.
We need to expose info API on api management? I don't see any other info API configured on api management.

Copy link
Contributor

Choose a reason for hiding this comment

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

If we don't have the need to avoid a conflict on the /info path (read: if we don't want to expose the API though an API managament) then you can strip down the prefix

@gunzip gunzip merged commit d3feb72 into master Nov 5, 2020
@gunzip gunzip deleted the 175496403-standard-health-check branch November 5, 2020 08:24
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.

5 participants