Add Advanced Sections to documentation #104

shaydewael · 2020-09-24T23:46:12Z

This adds advanced concepts to the documentation. I'm still working through some of the explanations for sections based on my understanding, but the code is in a state that's ready for review

Live preview: https://shaydewael.github.io/bolt-python/concepts

Category (place an `x` in each of the `[ ]`)

slack_bolt.App and/or its core components
slack_bolt.async_app.AsyncApp and/or its core components
Adapters in slack_bolt.adapter
Document pages under /docs
Others

Requirements (place an `x` in each `[ ]`)

Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you are agreeing to those rules.

I've read and understood the Contributing Guidelines and have done my best effort to follow them.
I've read and agree to the Code of Conduct.
I've run ./scripts/install_all_and_run_tests.sh after making the changes.

shaydewael · 2020-09-24T23:50:29Z

docs/_advanced/authorization.md

+- **`user_id`** only when using `user_token`.
+</div>
+
+```python


This is the only sample I haven't run locally because I couldn't get the version with the authorize function added. I'm guessing there's some mistakes. I will test this code sample and fix it in the near future.

shaydewael · 2020-09-24T23:51:23Z

docs/assets/style.css

  margin: 1em 0 0 33%;
  padding-bottom: 2em;
  font-size: 1em;
-  line-height: 1.5em;


There's some funky line-height stuff happening. I don't know why this was ever 1.5 because that's not how other docs are.

Ooof it's still doing it when it builds on github pages. I'll look into styling

@shaydewael Sorry, I didn't mention this but I made the changes. I'd like to address this line-spacing issue in the tutorial docs. I'm not sure why it doesn't happen to Bolt for JS. Can you adjust this or keep this change as-is?

The issue

Current

Ooof it's still doing it when it builds on github pages. I'll look into styling

@shaydewael I have been observing the configuration of Jekyll in this project may have some issues. Sometimes I have to delete _site directory to completely refresh the pages.

Hey Kaz - I changed a lot of styling stuff when I started messing around with this. One thing I learned - the stylesheet is being included from slack.dev rather than shaydewael.github.io. This is because it's using the url from the config file.

I added line numbers and adjusted spacing:

and the getting started page:

This is great! Thanks a lot.

codecov-commenter · 2020-09-24T23:54:54Z

Codecov Report

Merging #104 into main will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #104   +/-   ##
=======================================
  Coverage   90.03%   90.03%           
=======================================
  Files         135      135           
  Lines        3842     3842           
=======================================
  Hits         3459     3459           
  Misses        383      383

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 de93088...a96518a. Read the comment docs.

seratch

@shaydewael Thanks a lot for the amazing work! I really appreciate your efforts to polish the existing ones (including my English 🙏 ).

docs/_advanced/adapters.md

docs/_advanced/async.md

docs/_advanced/authorization.md

docs/_advanced/lazy_listener.md

seratch · 2020-09-25T02:09:33Z

docs/_advanced/lazy_listener.md

+
+However, apps running on FaaS or similar runtimes that don't allow you to run threads or processes after returning an HTTP response cannot follow this pattern. Instead, you should set the `process_before_response` flag to `true`. This allows you to call ack() and handle the event in seperate threads or processes. For events that do not require acknowledgement, you can create the listener as you normally would.
+
+Rather than acting as a decorator, lazy listeners take two keyword args:


Then, we can mention why you should use lazy in the case. The original one says To deal with this possible timeout issue, you can use ackandlazy functions for a listener. but something similar is fine.

Is this accurate now?

Thanks, it looks almost done. I added a suggestion above.

docs/_advanced/lazy_listener.md

docs/_advanced/logging.md

seratch · 2020-09-25T02:14:18Z

docs/_basic/listening_responding_shortcuts.md

 ```python

-# The open_modal shortcut opens a plain old modal
+# The open_modal shortcut listens to a shortcut with the callback_id "open_modal"


good catch 👍

Co-authored-by: Kazuhiro Sera <seratch@gmail.com>

docs/_advanced/authorization.md

seratch · 2020-09-28T20:58:07Z

docs/_advanced/authorization.md

+]
+
+def authorize(enterprise_id, team_id, logger):
+    logger.info(f"{enterprise_id},{team_id}")


We can remove this line

docs/_advanced/authorization.md

docs/_advanced/custom_adapters.md

seratch · 2020-09-28T21:16:10Z

docs/_advanced/lazy_listener.md

+
+Typically you'd call `ack()` as the first step of your listener functions. Calling `ack()` tells Slack that you've received the event and are handling it in within reasonable amount of time (3 seconds).
+
+However, apps running on FaaS or similar runtimes that don't allow you to run threads or processes after returning an HTTP response cannot follow this pattern. Instead, you should set the `process_before_response` flag to `True`. This allows you to create a listener that calls `ack()` and handles the event in seperate threads or processes, though you still need to complete everything within 3 seconds. For events that do not require acknowledgement, you can create listeners as you normally would.


How about changing this way? Technically, it uses a thread internally but the more important point here is that process_before_response guarantees the completion of all the processes in a listener. Also, please polish the sentence for Events API. (NOTE: this is not Bolt Python specific - Bolt JS has the same limitation)

Suggested change

However, apps running on FaaS or similar runtimes that don't allow you to run threads or processes after returning an HTTP response cannot follow this pattern. Instead, you should set the `process_before_response` flag to `True`. This allows you to create a listener that calls `ack()` and handles the event in seperate threads or processes, though you still need to complete everything within 3 seconds. For events that do not require acknowledgement, you can create listeners as you normally would.

However, apps running on FaaS or similar runtimes that don't allow you to run threads or processes after returning an HTTP response cannot follow this pattern. Instead, you should set the `process_before_response` flag to `True`. This allows you to create a listener that calls `ack()` and handles the event safely, though you still need to complete everything within 3 seconds. For events, while a listener doesn't need `ack()` method call as you normally would, the listener needs to complete within 3 seconds, too.

seratch · 2020-09-28T21:17:05Z

docs/_advanced/lazy_listener.md

+
+However, apps running on FaaS or similar runtimes that don't allow you to run threads or processes after returning an HTTP response cannot follow this pattern. Instead, you should set the `process_before_response` flag to `true`. This allows you to call ack() and handle the event in seperate threads or processes. For events that do not require acknowledgement, you can create the listener as you normally would.
+
+Rather than acting as a decorator, lazy listeners take two keyword args:


Thanks, it looks almost done. I added a suggestion above.

docs/_advanced/listener_middleware.md

docs/_advanced/logging.md

docs/_basic/publishing_views.md

seratch · 2020-09-28T21:24:12Z

@shaydewael I left a few comments. Once you resolve those, we can merge these documents. Thanks for your amazing work here 👍

Co-authored-by: Kazuhiro Sera <seratch@gmail.com>

seratch

Thanks - all looks great. We can squash commits and merge now.

Shane DeWael added 5 commits September 23, 2020 12:20

Start advanced concepts

1266f9a

custom adapter

7017267

Add rest of advanced sections

866a243

Fix the very broken code

89dade4

Merge branch 'main' into add-advanced-concept-sections

9fbfb82

shaydewael marked this pull request as draft September 24, 2020 23:47

shaydewael commented Sep 24, 2020

View reviewed changes

Correct listener sections

70def19

shaydewael added the docs Improvements or additions to documentation label Sep 25, 2020

seratch reviewed Sep 25, 2020

View reviewed changes

shaydewael and others added 7 commits September 28, 2020 09:58

Update docs/_advanced/adapters.md

e15b549

Co-authored-by: Kazuhiro Sera <seratch@gmail.com>

Update docs/_advanced/adapters.md

0cd68c0

Co-authored-by: Kazuhiro Sera <seratch@gmail.com>

Update docs/_advanced/async.md

3865947

Co-authored-by: Kazuhiro Sera <seratch@gmail.com>

Update docs/_advanced/async.md

64ddf0c

Co-authored-by: Kazuhiro Sera <seratch@gmail.com>

Update docs/_advanced/custom_adapters.md

d6f09dd

Co-authored-by: Kazuhiro Sera <seratch@gmail.com>

Update docs/_advanced/lazy_listener.md

6dd417d

Co-authored-by: Kazuhiro Sera <seratch@gmail.com>

Update docs/_advanced/logging.md

cd01087

Co-authored-by: Kazuhiro Sera <seratch@gmail.com>

shaydewael requested review from misscoded and mwbrooks September 28, 2020 17:03

shaydewael and others added 9 commits September 28, 2020 10:14

Update docs/_advanced/authorization.md

2e91e5b

Co-authored-by: Kazuhiro Sera <seratch@gmail.com>

Update docs/_advanced/lazy_listener.md

5dc4e29

Co-authored-by: Kazuhiro Sera <seratch@gmail.com>

snake

f68b974

kaz edits

eae25ea

Fix enterprise_id

4367f6c

styling changes

3ae1fc6

unnecessary margin

4c8d42c

small sizing adjustments

880684c

a little branding

288c778

Merge branch 'main' into add-advanced-concept-sections

eb26cd1

shaydewael marked this pull request as ready for review September 28, 2020 21:22

seratch approved these changes Sep 28, 2020

View reviewed changes

shaydewael and others added 8 commits September 28, 2020 14:36

Update docs/_advanced/authorization.md

968f35c

Co-authored-by: Kazuhiro Sera <seratch@gmail.com>

Update docs/_advanced/authorization.md

a655db4

Co-authored-by: Kazuhiro Sera <seratch@gmail.com>

Update docs/_advanced/authorization.md

26b7ce8

Co-authored-by: Kazuhiro Sera <seratch@gmail.com>

Update docs/_advanced/custom_adapters.md

d893c40

Co-authored-by: Kazuhiro Sera <seratch@gmail.com>

Update docs/_advanced/listener_middleware.md

43b22e1

Co-authored-by: Kazuhiro Sera <seratch@gmail.com>

Update docs/_advanced/logging.md

4d10d29

Co-authored-by: Kazuhiro Sera <seratch@gmail.com>

Update docs/_basic/publishing_views.md

1db34ad

Co-authored-by: Kazuhiro Sera <seratch@gmail.com>

Kaz edits

a96518a

shaydewael requested a review from seratch September 28, 2020 21:41

seratch approved these changes Sep 28, 2020

View reviewed changes

shaydewael merged commit 42fa6a9 into slackapi:main Sep 28, 2020


		However, apps running on FaaS or similar runtimes that don't allow you to run threads or processes after returning an HTTP response cannot follow this pattern. Instead, you should set the `process_before_response` flag to `true`. This allows you to call ack() and handle the event in seperate threads or processes. For events that do not require acknowledgement, you can create the listener as you normally would.

		Rather than acting as a decorator, lazy listeners take two keyword args:


		Typically you'd call `ack()` as the first step of your listener functions. Calling `ack()` tells Slack that you've received the event and are handling it in within reasonable amount of time (3 seconds).

		However, apps running on FaaS or similar runtimes that don't allow you to run threads or processes after returning an HTTP response cannot follow this pattern. Instead, you should set the `process_before_response` flag to `True`. This allows you to create a listener that calls `ack()` and handles the event in seperate threads or processes, though you still need to complete everything within 3 seconds. For events that do not require acknowledgement, you can create listeners as you normally would.

Add Advanced Sections to documentation #104

Add Advanced Sections to documentation #104

Uh oh!

Conversation

shaydewael commented Sep 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Category (place an x in each of the [ ])

Requirements (place an x in each [ ])

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

The issue

Current

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shaydewael Sep 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Sep 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

seratch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

seratch commented Sep 28, 2020

Uh oh!

seratch left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

shaydewael commented Sep 24, 2020 •

edited

Loading

Category (place an `x` in each of the `[ ]`)

Requirements (place an `x` in each `[ ]`)

shaydewael Sep 28, 2020 •

edited

Loading

codecov-commenter commented Sep 24, 2020 •

edited

Loading