Skip to content

fix: Slack log service#2537

Merged
johanneskoester merged 8 commits intosnakemake:mainfrom
vincentczhou:slack_log_service
Dec 13, 2023
Merged

fix: Slack log service#2537
johanneskoester merged 8 commits intosnakemake:mainfrom
vincentczhou:slack_log_service

Conversation

@vincentczhou
Copy link
Copy Markdown
Contributor

@vincentczhou vincentczhou commented Dec 7, 2023

Description

This updates the Slack --log-service feature added in #170.
The Slacker package is no longer maintained, and legacy slack tokens are no longer generatable.
I've replace it with the officially supported Slack SDK (see setup.cfg), preserving the original functionality with minimal adjustments.

This has been minimally tested for functionality. However, the original implementation has a couple issues (which this PR does not address) -

  1. Lack of docs/ on how to set up the --log-service slack feature via Slack
  2. Logic is not robust - e.g. when msg["done"] == msg["total"], this does not mean the workflow is complete (checkpoints)
  3. The implementation of --log-service slack is synchronous, meaning that Snakemake workflows will wait for the implemented log_handler function to finish. For normal print statements, this isn't an issue, but repeated calls of Slack WebAPI's postMessage can significantly slow things down. Depending on the workflow (Snakefile), this can become noticeable.
  4. The implementation does not comply with Slack WebAPI's rate limiting.

The current implementation will look like this - messages on slack are sent by the user, using a user OAuth token.
image

I've already addressed points 2, 3, and 4 in a private repository using --log-handler that should be integrable into this current implementation, but it has not been robustly tested (integration can happen at a later PR).
image

Thanks!

QC

  • The PR contains a test case for the changes or the changes are already covered by an existing test case.
  • The documentation (docs/) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake).

@vincentczhou vincentczhou changed the title Slack log service fix: Slack log service Dec 7, 2023
@johanneskoester
Copy link
Copy Markdown
Contributor

Cool! Log handlers like this are also candidates for turning them into plugins. But I first have to design the interface for that. Of course, if you are interested, you could also give this a try. One would basically follow the same approach as with snakemake-interface-storage-plugins. We can however merge this first. May I ask you to format the code with black?

@vincentczhou
Copy link
Copy Markdown
Contributor Author

Thank you Dr. Köster! Sorry about the linting, it must have gotten messed up when I made the kwarg revision on github earlier. The plugin idea sounds great (especially for the integration of the point I made above) - would definitely be interested in exploring that further when I'm able to!

@johanneskoester johanneskoester enabled auto-merge (squash) December 13, 2023 10:21
@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@johanneskoester johanneskoester merged commit 26eb4ba into snakemake:main Dec 13, 2023
johanneskoester added a commit that referenced this pull request Dec 20, 2023
🤖 I have created a release *beep* *boop*
---


##
[8.0.0](v7.32.2...v8.0.0)
(2023-12-20)



### ⚠ BREAKING CHANGES

Snakemake 8 marks the beginning of decomposing Snakemake into a
framework of plugins. This enables the democratization of method
development within the Snakemake ecosystem.
We start with plugins for storage and execution backends. In the future,
there will be plugins for the scheduling, metadata, software deployment,
reporting, and many more.
This way, it will be possible to easily launch and explore new
developments in workflow management and reproducible data analysis
without the need to get your work merged into the main codebase of
Snakemake and also without the need to develop a new workflow management
system as a proof of concept.

In detail, Snakemake 8 introduces the following changes. Unfortunately
it was unavoidable to break some usages (we apologize).
Nevertheless, we tried to ensure that every removed or modified feature
has been replaced with an equivalent reimplementation, as outlined in
our [migration
docs](https://snakemake.readthedocs.io/en/latest/getting_started/migration.html#migrating-to-snakemake-8).
While Snakemake 8 has an even more thorough testing framework than any
release before, and while it has been quite heavily tested in practice
by us, you might initially experience bugs and glitches for which we
want to apologize beforehand.
We think that the massive codebase improvements are worth it in the long
run, and hope that everything goes well.
As always, any pull requests with test cases and pointers to bugs are
more than welcome.

#### Detailed breaking changes

* removed the long time ago deprecated support for dynamic, version, and
subworkflow (see [the migration
docs](https://snakemake.readthedocs.io/en/latest/getting_started/migration.html#migrating-to-snakemake-8))
* migrated old remote providers into storage plugins (see [the migration
docs](https://snakemake.readthedocs.io/en/latest/getting_started/migration.html#migrating-to-snakemake-8))
* migrated execution backends into plugins, including a change in the
respective command line interfaces (see [the migration
docs](https://snakemake.readthedocs.io/en/latest/getting_started/migration.html#migrating-to-snakemake-8))
* deprecates `--use-conda` and `--use-singularity` in favor of
`--software-deployment-method conda` or `--software-deployment-method
apptainer` and `--software-deployment-method conda apptainer` (see [the
migration
docs](https://snakemake.readthedocs.io/en/latest/getting_started/migration.html#migrating-to-snakemake-8))
* profile support is now versioned, such that different profiles can be
written for different minimum Snakemake versions (see [the migration
docs](https://snakemake.readthedocs.io/en/latest/getting_started/migration.html#migrating-to-snakemake-8))
* redesigned Snakemake API. It now uses a modern, dataclass based
approach (see [the migration
docs](https://snakemake.readthedocs.io/en/latest/getting_started/migration.html#migrating-to-snakemake-8))

### Features

* add ability to inject conda environments into running Snakefile
([#2479](#2479))
([6140e29](6140e29))
* add functionality for deploying sources if no shared FS is assumed
([#2486](#2486))
([76eac3c](76eac3c))
* add option to control software deployment mode (shared or non shared
FS) ([#2525](#2525))
([04ec2c0](04ec2c0))
* allow detailed configuration of shared FS usage
([#2528](#2528))
([0d34be9](0d34be9))
* allow environment variables in string values of profile (e.g. paths
may now contain elements like $USER).
([58dc70c](58dc70c))
* allow python expressions in --set-resources
([#2521](#2521))
([022a31e](022a31e))
* allow to set latency_wait in executor test suite
([c0bca0b](c0bca0b))
* automatically upload workflow sources to default storage provider if
no shared FS is used
([a450c49](a450c49))
* Faster ci test setup
([#2489](#2489))
([4798e8a](4798e8a))
* implement precommand
([#2482](#2482))
([ff0f979](ff0f979))
* redesigned Snakemake API. It now uses a modern, dataclass based
approach ([#2403](#2403))
([2be3bfa](2be3bfa))
* support for external executor plugins
([#2305](#2305))
([c9eaa4e](c9eaa4e))
* version specific profile config files (profile/config.v8+.yaml with
profile/config.yaml as fallback that matches any version)
([#2498](#2498))
([47e5811](47e5811))

### Bug Fixes

* adapt to changes in snakemake-interface-executor-plugins
([635c68a](635c68a))
* add storage provider args to deploy sources command
([67178e3](67178e3))
* add testcase for script directive to work with Python 3.7 and
corresponding fix.
([0b4ae2e](0b4ae2e))
* allow pepfile and pepschema to take pathlib
([#2546](#2546))
([ca91661](ca91661))
* also inherit rule proxies if there is no rulename modifier specified
in a use rule statement
([#2440](#2440))
([1570289](1570289))
* assume at most 8GB memory for default resources. This way, we avoid
exploding memory requirements for large input files that are very
unlikely to be put entirely into memory by any tool.
([11c2ecc](11c2ecc))
* comparison to float in scheduler
([ef44d84](ef44d84))
* detect job paths that leave and then enter a group. Such paths are
invalid because then the group depends on itself.
([#2527](#2527))
([5383a4d](5383a4d))
* ensure that auto deployment of default storage provider works in
containers with read-only root home.
([1a347ff](1a347ff))
* ensure that log and benchmark files are uploaded to storage as well
([#2545](#2545))
([6aabb5d](6aabb5d))
* ensure that targetjob is always forced. This fixes a bug causing
run-directive rules to not being executed even when enforced via e.g.
-R. ([#2448](#2448))
([b2a60d5](b2a60d5))
* fix cache handling and unlock handling
([2f4d5e1](2f4d5e1))
* fix nargs definition for --deploy-sources
([fc252c8](fc252c8))
* fix path handling when detective profiles
([fe63881](fe63881))
* fix storage handling on windows by converting all paths to posix paths
([#2519](#2519))
([7864a76](7864a76))
* handle different f-string tokens in py3.12
([#2485](#2485))
([f2c7613](f2c7613))
* handle storage for local jobs; add test case
([6d978ef](6d978ef))
* handling of group jobs when obtaining temp input files
([71be1de](71be1de))
* import ([#2402](#2402))
([2c831f1](2c831f1))
* improved error handling for storage upload; fixed bugs caused by
outdated calls to IOFile.exists().
([720bb84](720bb84))
* improved error messages in case of invalid storage queries
([9671fd0](9671fd0))
* in addition to localrules statement, infer that job is local if it has
any input or output file that is marked as local
([#2541](#2541))
([e8b682b](e8b682b))
* only deactivate conda inject envs upon workflow tear down
([#2503](#2503))
([e6dfdd4](e6dfdd4))
* Panoptes --wms-monitor-arg
([#2444](#2444))
([98d2bdf](98d2bdf))
* proper reuse of rule proxies when importing several times from the
same module
([#2404](#2404))
([e867dda](e867dda))
* Restore backward compatibility for Google Life Sciences executor
([#2461](#2461))
([5e3a464](5e3a464))
* shadow "full" mode ignore symlinks
([#2516](#2516))
([1d58120](1d58120))
* show failed logs in executor testcases
([92f7bf4](92f7bf4))
* Slack log service
([#2537](#2537))
([26eb4ba](26eb4ba))
* sort report (sub-)categories in lexicographical order
([#2449](#2449))
([d0705ad](d0705ad))
* update minimum snakemake-interface-storage-plugins version
([0ef7226](0ef7226))
* use temporary directory (faster, more likely local, always writable)
for persistence and source cache in case of remote execution without
shared fs ([#2502](#2502))
([c8fa7ba](c8fa7ba))
* wait for logs before showing them on error
([a4ff328](a4ff328))


### Documentation

* document name directive with example
([#2534](#2534))
([cce5551](cce5551))
* fix syntax in cluster example
([#2460](#2460))
([64e9645](64e9645))
* notes on arm based machines in tutorial docs
([0586f04](0586f04))
* **rust:** Fix typo on rust-script version
([#2488](#2488))
([a79dd94](a79dd94))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Johannes Köster <[email protected]>
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