-
Notifications
You must be signed in to change notification settings - Fork 6
DVC 7404: Add linter action #17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
example/example.py
Outdated
logging.exception("Exception when calling DevcycleApi->all_features: %s\n" % e) | ||
|
||
key = 'elliot-test' # str | Variable key | ||
variable_key = os.getenv('DVC_VARIABLE_KEY') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure if we want to encourage folks to load variable keys from environment variables, as I don't think that's a workflow we would expect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point - I made this change here and in the Go examples just as a first step to get away from "elliot-test", and make it easy to use whatever variable you'd already created.
Do you think using a generic variable name along with instructions to create it would be better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the two competing goals here are:
- showing the SDK being used the way we intend
- the sample app just works the first time you try it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made a bunch of updates to this file that I think make it a bit more idiomatic:
https://github.com/DevCycleHQ/python-server-sdk/blob/83e4c78dd39018c26271aaad1e3604b00118f6c7/example/example.py
tl;dr:
- Hardcoded variable name that can be created easily without overlapping with existing ones
- Get the SDK key from an env var and log an obvious message if it's not set
- Use both
variable_value
andvariable
without a try/catch and with a conditional to show realistic usage - Print out a hint if the sample variable doesn't seem to exist
- only wrap all the other methods with one try/catch to make things a bit more readable - this should only trigger if our API is unavailable anyways.
I think there's an opportunity here to update all the SDK examples to be a quick interactive intro to DevCycle, instead of just dumping out a bunch of variable metadata.
Adds a linter action using https://github.com/charliermarsh/ruff, turns on a few basic rules, and fixes the errors that turns up so we can start off with the action being required.
This one should be pretty dev friendly since it uses a config file that affects both the Github action and the local tool, plus the errors actually show up in the action output.