bitwarden / splunk

Splunk app for reporting Bitwarden event logs.
GNU General Public License v3.0
14 stars 8 forks source link

Improve readme #54

Closed eliykat closed 2 months ago

eliykat commented 3 months ago

🎟️ Tracking

N/A

📔 Objective

I set up the Splunk app for local development today and hit a few pain points, possibly because I'm not familiar with the Python ecosystem. In particular:

I've added these to the instructions, and also restructured them to be more step-by-step.

Two follow-up questions:

  1. are there any objections to me moving these instructions to https://contributing.bitwarden.com where the rest of our documentation lives?
  2. pyproject.toml specifies the python version as python = ">=3.7.17", however these instructions refer to Python 3.8 specifically. I received syntax errors when trying to use 3.12, which is what I already had installed on my system. I suggest we probably need to specify ^3.8 - what do you think @WaciX ?

⏰ Reminders before review

🦮 Reviewer guidelines

github-actions[bot] commented 3 months ago

Logo Checkmarx One – Scan Summary & Details2b06cd54-ccbc-4387-91a3-932036b34ac1

No New Or Fixed Issues Found

eliykat commented 3 months ago

Thanks @mzieniukbw , I've continued these changes here: https://github.com/bitwarden/contributing-docs/pull/386

Once that's merged, we can just update this doc to link there.

Re: the Python version - can we specify that it should be =< 3.9? I was more concerned about limiting the use of newer, incompatible versions (3.12 threw errors) rather than bumping the min version (although I see that is what I wrote).

mzieniukbw commented 3 months ago

@eliykat i have just tested other python versions and added a suggestion in https://github.com/bitwarden/contributing-docs/pull/386/files 3.10 works fine, but 3.11 and 3.12 throws errors during package validation (splunk tooling python incompatibility). I don't want to limit the users to specific python versions, so giving them a range, like 3.7-3.10 is fine. We could limit it in the poetry pyproject.toml file, i don't mind.

eliykat commented 2 months ago

I like specifying it in pyproject.toml, it's an easy thing to miss and it's better to get an explicit error than some random runtime failure. I've done that and regenerated the lockfile as required. Now when I try with 3.12:

Current Python version (3.12.4) is not allowed by the project (>=3.7.17, <=3.10). Please change python executable via the "env use" command.

I've also updated the README to point to the contributing docs.