Workflow steps decorator by seratch · Pull Request #113 · slackapi/bolt-python

seratch · 2020-10-04T08:09:04Z

This pull request adds Workflow Steps support using decorator interface. This way provides more Pythonic way of coding and more flexibility for workflow step listeners. Adding this feature doesn't break anything for the default way to add steps to Bolt apps.

app = App()
copy_review_step = WorkflowStep.builder("copy_review")

@copy_review_step.edit
def edit(ack: Ack, step, configure: Configure):
    ack()
    configure(blocks=[])

@copy_review_step.save
def save(ack: Ack, step: dict, view: dict, update: Update):
    state_values = view["state"]["values"]
    update(inputs={}, outputs=[])
    ack()

def additional_matcher(step):
    return True

def noop_middleware(next):
    return next()

def notify_execution(client: WebClient, step: dict):
    time.sleep(5)
    client.chat_postMessage(channel="#random", text=f"Step execution: ```{step}```")

@copy_review_step.execute(
    matchers=[additional_matcher],
    middleware=[noop_middleware],
    lazy=[notify_execution],
)
def execute(step: dict, client: WebClient, complete: Complete, fail: Fail):
    try:
        complete(outputs={})
    except Exception as err:
        fail(error={"message": f"Something wrong! {err}"})

app.step(copy_review_step)

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.

seratch

Comments for reviewers. I won't merge this PR without approvals from other maintainers.

seratch · 2020-10-04T08:10:05Z

                callback_id=callback_id, edit=edit, save=save, execute=execute,
            )
+        elif isinstance(step, WorkflowStepBuilder):
+            step = step.build()


The build method validates the state of the WorkflowStepBuilder object.

seratch · 2020-10-04T08:12:16Z

+        def edit_my_step(ack, configure):
+            pass
+        """
+        if len(args) == 1:


This is the pattern for @my_step.edit (= the edit method is used without ()).

codecov-commenter · 2020-10-04T12:42:37Z

Codecov Report

Merging #113 into main will decrease coverage by 0.50%.
The diff coverage is 81.94%.

@@            Coverage Diff             @@
##             main     #113      +/-   ##
==========================================
- Coverage   90.65%   90.14%   -0.51%     
==========================================
  Files         148      148              
  Lines        4141     4375     +234     
==========================================
+ Hits         3754     3944     +190     
- Misses        387      431      +44

Impacted Files	Coverage Δ
slack_bolt/app/app.py	`85.05% <77.77%> (+0.01%)`	⬆️
slack_bolt/app/async_app.py	`94.19% <77.77%> (-0.22%)`	⬇️
slack_bolt/workflows/step/step.py	`85.54% <82.17%> (-8.11%)`	⬇️
slack_bolt/workflows/step/async_step.py	`85.63% <82.30%> (-8.02%)`	⬇️

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 b4bc3e1...6c606c4. Read the comment docs.

Ambro17 · 2020-10-07T17:59:05Z

+            pass
+        """
+
+        if len(args) == 1:


What do you think about delegating this conditional to a method named something like used_without_argument or maybe used_as_plain_decorator?

Eventuals refactors to this logic may be easier by changing it in only one place, we would need to pass the type of listener to that method

It sounds nice. I will have some method to commonize the logic. Thanks for the feedback!

Ambro17 · 2020-10-07T18:02:34Z

+            for sub in kwargs["lazy"]:
+                functions.append(sub)
+            return functions
+        return None


Is there any problem if we return [] instead of None?

Indeed. I agree we can return an empty list in the case 👍

seratch

@Ambro17 Thanks for your great feedback 🙇

seratch · 2020-10-07T22:16:50Z

+            pass
+        """
+
+        if len(args) == 1:


It sounds nice. I will have some method to commonize the logic. Thanks for the feedback!

seratch · 2020-10-07T22:17:42Z

+            for sub in kwargs["lazy"]:
+                functions.append(sub)
+            return functions
+        return None


Indeed. I agree we can return an empty list in the case 👍

mwbrooks

Really like all the examples that you took the time to include!

codecov-io · 2020-10-20T03:26:44Z

Codecov Report

Merging #113 into main will decrease coverage by 0.27%.
The diff coverage is 85.66%.

@@            Coverage Diff             @@
##             main     #113      +/-   ##
==========================================
- Coverage   91.08%   90.80%   -0.28%     
==========================================
  Files         148      149       +1     
  Lines        4184     4406     +222     
==========================================
+ Hits         3811     4001     +190     
- Misses        373      405      +32

Impacted Files	Coverage Δ
slack_bolt/app/app.py	`85.05% <77.77%> (+0.01%)`	⬆️
slack_bolt/app/async_app.py	`94.19% <77.77%> (-0.22%)`	⬇️
slack_bolt/workflows/step/step.py	`88.55% <86.06%> (-5.10%)`	⬇️
slack_bolt/workflows/step/async_step.py	`88.62% <86.17%> (-5.03%)`	⬇️
slack_bolt/workflows/step/internals.py	`100.00% <100.00%> (ø)`

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 4ecce6b...2248cf8. Read the comment docs.

seratch · 2020-10-21T03:20:42Z

@aoberoi told me that he was going to share some thoughts on this and that's the reason why I'm holding off merging this PR. I will change the milestone for this from 0.9 to 1.1 for now.

seratch · 2021-01-04T07:14:52Z

I am planning to update this PR for aiming to merge it in v1.3. Let me know if you have further comments or feedback on this.

seratch · 2021-01-27T05:17:27Z

We will use #224 for this

seratch added enhancement New feature or request area:async area:sync labels Oct 4, 2020

seratch added this to the 1.0.0 (GA) milestone Oct 4, 2020

seratch requested review from aoberoi, misscoded, mwbrooks and stevengill October 4, 2020 08:09

seratch self-assigned this Oct 4, 2020

seratch commented Oct 4, 2020

View reviewed changes

seratch modified the milestones: 1.0.0 (GA), 0.9.2b0 Oct 4, 2020

seratch modified the milestones: 0.9.2b0, 0.9.3b0 Oct 6, 2020

Ambro17 reviewed Oct 7, 2020

View reviewed changes

seratch commented Oct 7, 2020

View reviewed changes

mwbrooks approved these changes Oct 8, 2020

View reviewed changes

seratch modified the milestones: 0.9.4b0, 1.1.0 Oct 21, 2020

seratch mentioned this pull request Oct 25, 2020

Enable WorkflowStep to have lazy listeners #127

Merged

5 of 8 tasks

seratch added 2 commits October 26, 2020 07:56

Add decorator based interface for Workflow Steps

f4dc58a

Update the code

2248cf8

seratch modified the milestones: 1.1.0, 1.3.0 Nov 25, 2020

seratch mentioned this pull request Jan 27, 2021

Add Workflow step decorator #224

Merged

5 of 8 tasks

seratch closed this Jan 27, 2021

seratch mentioned this pull request Feb 4, 2021

version 1.3.0 release #230

Merged

Conversation

seratch commented Oct 4, 2020

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

Requirements (place an x in each [ ])

Uh oh!

seratch left a comment

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!

codecov-commenter commented Oct 4, 2020

Codecov Report

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

Uh oh!

seratch left a comment

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!

mwbrooks left a comment

Choose a reason for hiding this comment

Uh oh!

codecov-io commented Oct 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

seratch commented Oct 21, 2020

Uh oh!

seratch commented Jan 4, 2021

Uh oh!

seratch commented Jan 27, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

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

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

codecov-io commented Oct 20, 2020 •

edited

Loading