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 ngrok sidecar to facebook webhook #126

Merged
merged 9 commits into from
Oct 22, 2020
Merged

Add ngrok sidecar to facebook webhook #126

merged 9 commits into from
Oct 22, 2020

Conversation

ljupcovangelski
Copy link
Contributor

No description provided.

@@ -1,13 +1,15 @@
#!/bin/bash
set -euo pipefail
IFS=$'\n\t'
#set -euo pipefail
Copy link
Contributor

Choose a reason for hiding this comment

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

why did you comment the strict mode out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It failed for me for unbound-variables and I couldn't solve it otherwise. But we can do some more testing and try ti keep it.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah yes that is what the u does. so you could just remove that

@@ -17,13 +19,17 @@ do
fi
done < ../airy.conf

sed -i '.bak' "s/<fb_app_id>/${config[FB_APP_ID]}/" ../deployments/sources-facebook-events-router.yaml
sed -i '.bak' "s/<fb_app_id>/${config[FB_APP_ID]}/" ../deployments/api-admin.yaml
sed -i "s/<fb_app_id>/${config[FB_APP_ID]}/" ../deployments/sources-facebook-events-router.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

it doesn't work without a backup file on mac. At least not with sed

Copy link
Contributor

Choose a reason for hiding this comment

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

-i '' it seems to work with an empty string

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah... but note that this is run inside the vagrant box, so it runs on LInux and not on Mac

Copy link
Contributor

@lucapette lucapette left a comment

Choose a reason for hiding this comment

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

First pass! to be fair I only looked at the docs!

README.md Outdated
$ vagrant ssh
$ kubectl get pods
```

When the platform starts it will print out the public webhook urls and the local API IP and port.
Copy link
Contributor

Choose a reason for hiding this comment

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

We should have this into a paragraph (so it's easier to reach from the top of the document).

will is almost never a good idea in technical documentation. The same sentence without the will reads better. Even better would be:

During the bootstrapping phase, the Airy Core Platform prints out the public webhook URLs and the local address you can use to query the API.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done 👍

README.md Outdated
$ vagrant ssh
$ kubectl get pods
```

When the platform starts it will print out the public webhook urls and the local API IP and port.
To refresh this information, you can run:
Copy link
Contributor

Choose a reason for hiding this comment

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

refresh as recreate? reload? I can't say

Copy link
Contributor Author

Choose a reason for hiding this comment

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

View again. Maybe refetch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I put

To display this information again, you can run:

Also print can be used I think.

README.md Outdated
$ /vagrant/scripts/status.sh
```

You can stop, start or restart the Airy Core Platform virtual machine with the following commands
Copy link
Contributor

Choose a reason for hiding this comment

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

the document is using box instead of virtual machine (I"m fine with both but we have to stay consistent). Let's also add a : at the end of the sentence

README.md Outdated
$ vagrant reload
```

You can delete and re-create the whole environment with the following commands
Copy link
Contributor

Choose a reason for hiding this comment

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

:

README.md Outdated

### Connect the Facebook source

In order to work with the Facebook source, you will need to provide your Facebook credentials. The system will provision a demo credentials initially and if you want to put in your facebook credentials, please edit the `infrastructure/airy.conf` config file and then run the following command
Copy link
Contributor

Choose a reason for hiding this comment

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

will can be removed everywhere. Also :.

please is also not very good for technical documentation. In this case you must is correct (because the user has to do it)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe you need to ? Because he should do it because he needs it, maybe must is too implying?

docs/glossary.md Show resolved Hide resolved
@@ -38,6 +38,12 @@ Since Minikube clusters are usually not exposed to the public internet, we
included an ngrok client to facilitate the integration of sources (via
webhooks).

In order for the Airy Core Platform to be accessible from the outside (for example from Facebook, in order to send events to the Facebook webhook),
Copy link
Contributor

Choose a reason for hiding this comment

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

have included -> included

, which will create -> which creates (notice the comma also)

The Airy Core Platform prints out the public URL (active voice > passive voice)

@ljupcovangelski ljupcovangelski merged commit a83cf48 into main Oct 22, 2020
@ljupcovangelski ljupcovangelski deleted the infra/ngrok branch October 22, 2020 10:02
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