search

reportportal / agent-python-pytest

Framework integration with PyTest
Apache License 2.0
94 stars 102 forks source link

Add support for reading rp_endpoint from environment variable #270

Closed subecho closed 3 years ago

subecho commented 3 years ago

Currently, the only way to set the ReportPortal endpoint is through the pytest.ini file. The patch adds another way by looking for the environment variable "RP_ENDPOINT" before looking in the pytest.ini file, similar to how the RP_UUID environment variable works.

subecho commented 3 years ago

"Dirty Hack" seems a bit harsh, especially to a first time contributor, and when that "hack" was based off of prior art in the same file for rp_uuid it leaves me with a bad taste in my mouth. How is what I'm doing with the endpoint different than what was already being done for the UUID?

I read through CONTRIBUTING.rst and followed the instructions there, running the unit tests and checks before creating the PR and ensuring they pass, but there was no reference to the contributor guide linked above. Had there been, I would have followed it.

I will add unit tests when I can, but my excitement for doing so (and for further contributions) has been soured by the response here to a first time contributor who had the audacity to contribute something that helped them and figured the community would also like the same benefit.

HardNorth commented 3 years ago

@subecho

that "hack" was based off of prior art in the same file for rp_uuid

That only means rp_uuid was a hack too. What about other properties? Do they not worth being passed through environment variables?

Thanks for pointing on the problem, but I can't accept the way you solve it.

subecho commented 3 years ago

Very well then. Good luck fixing it and thanks for the feedback.

iivanou commented 3 years ago

rp_uuid, ep_endpoint, and other variables can be implemented as command-line options in addition to the ability to set those in the *.ini file. However, there is a reason why rp_uuid was implemented in that way. In some cases, CI/CD configuration files may include command-line actions where rp_uuid can be disclosed. From a security point, it's better to keep it in env vars. I do not see any limitations for other parameters.

I agree with @HardNorth in this case. Thanks for MR, @subecho.