Add more documentation around contructor parameters

[filmaj]

Summary

Parameters for client, async_client and legacy_client as well as their base classes. In particular focusing on documenting the proxy and ssl parameters (relates to #1214).

Preview

web.client expanded constructor parameter list that now includes ssl, proxy and headers:

web.base_client with instance variables now documented:

Similar changes for web.async_client, with the addition of session and trust_env_in_session parameters:

Similar to the above changes also exist in the legacy client, base legacy client, and base async client.

Questions / TODOs

• I did not want to bundle the generated docs/ folder as first I want to make sure the changes make sense.
• Once approved I can push up a separate commit re-generating all the docs.
• @seratch what do you think about having the scripts/docs.sh script also calling into generate_api_docs.sh? This way a single command generates all the docs in one go.

[filmaj]

Hmm let me check the flake8 failures... the GitHub CI output is not very helpful... to be fair I only ran the tests on Python 3.9 so I will check now on my machine with Python 3.7.

[seratch]

@filmaj Thanks for improving this!

@seratch what do you think about having the scripts/docs.sh script also calling into generate_api_docs.sh? This way a single command generates all the docs in one go.

This could be fine but one reason we've been separating the two generation processes is that, when we want to correct only the documents without updating the API docs with the latest docstrings, merging the two can be an obstacle. Some changes in the latest revision may not be available until cutting a new release. To avoid confusion, we've been updating the API docs when releasing a new version.

For the above reason, I still recommend separating the process. If we want to merge them, we may want to add an argument like --skip-api-docs to the docs.sh script.

Does this make sense?

[filmaj]

@seratch yes makes sense, thanks for explaining!

[seratch]

@filmaj The failure may be an occasional issue on the GitHub Action runtime side. In the past, I've observed segmentation fault and so on that never happen on my local machine. If you rerun the job and it succeeds, it is just fine for now.

[filmaj]

Ah I think it is my mistake. The CI runs python setup.py validate and the output from there shows:

/home/runner/work/python-slack-sdk/python-slack-sdk/slack_sdk/web/async_client.py:56:126: E501 line too long (129 > 125 characters)

If I run python setup.py validate locally I can reproduce the issue and indeed that command exits with non-zero code. I was surprised as I assumed ./scripts/run_validation.sh would have caught that. I will fix by using a different markdown syntax.

[codecov]

Codecov Report

Merging #1215 (35f9e56) into main (da8c0a1) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1215      +/-   ##
==========================================
- Coverage   86.59%   86.58%   -0.01%     
==========================================
  Files         111      111              
  Lines       10943    10943              
==========================================
- Hits         9476     9475       -1     
- Misses       1467     1468       +1     

Impacted Files	Coverage Δ
slack_sdk/web/async_base_client.py	98.33% <ø> (ø)
slack_sdk/web/async_client.py	85.14% <ø> (ø)
slack_sdk/web/base_client.py	89.10% <ø> (ø)
slack_sdk/web/client.py	86.52% <ø> (ø)
slack_sdk/web/legacy_base_client.py	87.75% <ø> (ø)
slack_sdk/web/legacy_client.py	86.24% <ø> (ø)
slack_sdk/socket_mode/builtin/internals.py	72.36% <0.00%> (-0.44%)	⬇️

---

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 da8c0a1...35f9e56. Read the comment docs.

[filmaj]

@seratch sorry one more thing around generating docs during release time you mentioned:

To avoid confusion, we've been updating the API docs when releasing a new version.

In the maintainer's guide under the releasing steps, it says to only generate the docs using ./scripts/docs.sh. Perhaps I should update that to also mention running ./scripts/generate_api_docs.sh?

[seratch]

@filmaj good catch! We should add it 👍