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

Add a basic ping endpoint #39

Merged
merged 1 commit into from
Sep 8, 2017
Merged

Conversation

bdunne
Copy link
Member

@bdunne bdunne commented Aug 31, 2017

Allows us to know whether the API server is ready to serve requests

@@ -5,6 +5,8 @@
root :to => "api#index", :as => :entrypoint
match "/", :to => "api#options", :via => :options

get "/ping" => "ping#index"
Copy link
Member

Choose a reason for hiding this comment

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

/ping or /api/ping?

Copy link
Member

Choose a reason for hiding this comment

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

or both?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's under the api namespace, so it's /api/ping

@abellotti
Copy link
Member

I see it's based off ActionController::API and not BaseController, so the intent is to have it ignore authentication then ?

Other than that, being /api/ping, we can not have a collection in the future called "ping", I think that's ok 😏

If so, I'm good with this. maybe a simple test for it expecting the pong.

Thanks @bdunne

@bdunne
Copy link
Member Author

bdunne commented Sep 6, 2017

@abellotti

  • Correct, I didn't want authentication on this endpoint.
  • I thought about the collection, but I didn't think that would conflict. 😉

@abellotti
Copy link
Member

ping @bdunne 😄 can you add a small test when you get a chance ? Thanks.

@@ -0,0 +1,8 @@
describe "PingController" do
it "get" do
get("/api/ping")
Copy link
Member

Choose a reason for hiding this comment

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

minor, could you update to use the rails helper for consistency with the other specs.

run_get api_ping_url

Thanks,

Allows us to know whether the API server is ready to serve requests
@miq-bot
Copy link
Member

miq-bot commented Sep 8, 2017

Checked commit bdunne@01c8265 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
3 files checked, 0 offenses detected
Everything looks fine. 👍

@abellotti
Copy link
Member

Thanks @bdunne for updating the test. will merge when 🍏

@abellotti abellotti added this to the Sprint 69 Ending Sep 18, 2017 milestone Sep 8, 2017
@abellotti abellotti merged commit 4f54953 into ManageIQ:master Sep 8, 2017
@bdunne bdunne deleted the ping_endpoint branch September 8, 2017 16:27
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.

4 participants