Add missing-timeout warning for requests.Session object

[joel6948]

some issue fixes
#9517 for this issue

[Pierre-Sassoulas]

Thank you for your contribution @joel6948, do you mind expanding missing-timeout instead of creating a new extension please ?

[joel6948]

Thank you for your contribution @joel6948, do you mind expanding missing-timeout instead of creating a new extension please ?

sure

[joel6948]

Thank you for your contribution @joel6948, do you mind expanding missing-timeout instead of creating a new extension please ?

#9527 (comment)
done

[Pierre-Sassoulas]

Suggested change

	# Other checks...
	self._check_positional_only_arguments_expected(node)

This should stay, you can delete pylint/extensions/requests_timeout_plugin.py and add your use case in _check_missing_timeout directly (without adding a new message, just use the existing missing-timeout).

[joel6948]

done

[Pierre-Sassoulas]

I think you added a python file in the documentation, but you need to modify the code / add your logic here instead:

pylint/pylint/checkers/method_args.py

Lines 76 to 100 in 27b1ae7

	def _check_missing_timeout(self, node: nodes.Call) -> None:
	"""Check if the call needs a timeout parameter based on package.func_name
	configured in config.timeout_methods.
	Package uses inferred node in order to know the package imported.
	"""
	inferred = utils.safe_infer(node.func)
	call_site = arguments.CallSite.from_call(node)
	if (
	inferred
	and not call_site.has_invalid_keywords()
	and isinstance(
	inferred, (nodes.FunctionDef, nodes.ClassDef, bases.UnboundMethod)
	)
	and inferred.qname() in self.linter.config.timeout_methods
	):
	keyword_arguments = [keyword.arg for keyword in node.keywords]
	keyword_arguments.extend(call_site.keyword_arguments)
	if "timeout" not in keyword_arguments:
	self.add_message(
	"missing-timeout",
	node=node,
	args=(node.func.as_string(),),
	confidence=INFERENCE,
	)

Please note that we're using inference which can handle case where a variable is used and not request.Session directly.

[joel6948]

but in the main branch is it showing inference which can handle case where a variable is used and not request.Session directly. or is it my mistake

[Pierre-Sassoulas]

Hmm, not sure. I think we handle a call to request.get (and other see self.linter.config.timeout_methods) (directly or inferred) but not Session. It should be able to do both (?). Maybe the solution is as simple as changing the default for timeout_methods

[joel6948]

i did the change a bit can you is it working

[Pierre-Sassoulas]

One way to test this would be to create a functional tests, see here: https://pylint.readthedocs.io/en/stable/development_guide/contributor_guide/tests/writing_test.html#functional-tests and here https://github.com/pylint-dev/pylint/blob/main/tests/functional/m/missing/missing_timeout.py

[Pierre-Sassoulas]

You're also going to need to add a changelog entry, see:

create a news fragment with towncrier create . which will be included in the changelog. can be one of the types defined in ./towncrier.toml. If necessary you can write details or offer examples on how the new change is supposed to work.
https://pylint.readthedocs.io/en/stable/development_guide/contributor_guide/contribute.html#creating-a-pull-request

[github-actions]

🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉

This comment was generated for commit 9f99786