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

feature/implementing-ngrok-startup-script #21

Merged
merged 11 commits into from
Nov 17, 2022
Merged

feature/implementing-ngrok-startup-script #21

merged 11 commits into from
Nov 17, 2022

Conversation

anwalker293
Copy link
Contributor

Signed-off-by: Alexandra N. Walker alex.walker@indicio.tech ☀️

Signed-off-by: anwalker293 <alex.walker@indicio.tech>
Signed-off-by: anwalker293 <alex.walker@indicio.tech>
@anwalker293 anwalker293 marked this pull request as ready for review November 7, 2022 16:42
Signed-off-by: anwalker293 <alex.walker@indicio.tech>
WAIT_INTERVAL=${WAIT_INTERVAL:-3}
WAIT_ATTEMPTS=${WAIT_ATTEMPTS:-10}

retrieve_endpoint () {
Copy link
Member

Choose a reason for hiding this comment

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

This is more of a function that makes sure the server is ready rather than retrieving the endpoint.

retrieve_endpoint () {
for _ in $(seq 1 "$WAIT_ATTEMPTS"); do
#TODO If wait attempts exceeded and we still haven't successfully gotten an endpoint, exit with status 1
if ! curl -s -o /dev/null -w '%{http_code}' "${1}/url" | grep "200" > /dev/null; then
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if ! curl -s -o /dev/null -w '%{http_code}' "${1}/url" | grep "200" > /dev/null; then
if ! curl -s -o /dev/null -w '%{http_code}' "${1}" | grep "200" > /dev/null; then

I think we need to remove the /url so the function args can specify the endpoint fully and not have anything appended to it.


retrieve_endpoint () {
for _ in $(seq 1 "$WAIT_ATTEMPTS"); do
#TODO If wait attempts exceeded and we still haven't successfully gotten an endpoint, exit with status 1
Copy link
Member

Choose a reason for hiding this comment

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

I think we should implement this TODO. In the branch where we do the waiting, we should check if current number of attempt is equal to max and then exit 1 if it is.

Signed-off-by: anwalker293 <alex.walker@indicio.tech>
Signed-off-by: anwalker293 <alex.walker@indicio.tech>
Signed-off-by: anwalker293 <alex.walker@indicio.tech>
Signed-off-by: anwalker293 <alex.walker@indicio.tech>
Signed-off-by: anwalker293 <alex.walker@indicio.tech>
Signed-off-by: anwalker293 <alex.walker@indicio.tech>
Signed-off-by: Alex Walker <alex.walker@indicio.tech>
Also making requested changes

Signed-off-by: Alex Walker <alex.walker@indicio.tech>
@anwalker293
Copy link
Contributor Author

@dbluhm I think this is ready to rock and roll :)

Copy link
Member

@dbluhm dbluhm 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 to me!

@dbluhm dbluhm merged commit 9c75605 into Indicio-tech:feature/revocation Nov 17, 2022
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.

2 participants