-
Notifications
You must be signed in to change notification settings - Fork 0
Add Datadog Serverless Compatibility Layer #1
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
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.
LGTM
- id: package | ||
run: | | ||
PACKAGE_VERSION=$(awk -F '"' '/version = / {print $2}' pyproject.toml) | ||
echo "package-version=$PACKAGE_VERSION" >> "$GITHUB_OUTPUT" |
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.
So, it looks to me like this is grabbing the current version number. Meaning, that in order to do a proper release, you've gotta first bump the version number in the pyproject.toml file, then run the gh action.
I'm wondering if instead maybe we could pass the new package version in as an option? You'd then need the gh action to open a PR though. Maybe another idea is to confirm in the input options that the user knows that the pyproject.toml version must first be updated?
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.
So, it looks to me like this is grabbing the current version number. Meaning, that in order to do a proper release, you've gotta first bump the version number in the pyproject.toml file, then run the gh action.
That's correct
I'm wondering if instead maybe we could pass the new package version in as an option? You'd then need the gh action to open a PR though. Maybe another idea is to confirm in the input options that the user knows that the pyproject.toml version must first be updated?
Yeah I think some sort of validation is needed. I see the version for datadog-lambda-python
is bumped and committed prior to a release. Is it just a matter of documenting the release steps internally?
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 you get to define the release steps however you want. We certainly don't stick to a single way of doing it across all our repos. I think you should do what you think will be most appropriate for this repo. Just make sure to be explicit as to when you think the version should be bumped in the pyproject.toml file.
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.
does this workflow fail if we accidentally try to publish the same version twice?
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.
@apiarian-datadog According to the PyPI help docs, the same version will not be allowed to be published twice.

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.
- Remove Python 4 support
- Let's talk about release steps
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.
since we have tests, should we have some sort of test runner workflow? or do those need to be in gitlab?
also, might be worth adding some pre-commit-hooks to check formatting, etc.
datadog_serverless_compat/logger.py
Outdated
def initialize_logging(name): | ||
logger = logging.getLogger(name) | ||
|
||
str_level = (os.environ.get("DD_LOG_LEVEL") or "INFO").upper() |
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.
nit: os.environ.get("DD_LOG_LEVEL", "INFO")
usually? unless there's something more subtle going on with that or
statement?
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.
The behavior is slightly different when DD_LOG_LEVEL
is set to an empty string versus not being set (ie. None).
os.environ.get("DD_LOG_LEVEL") or "INFO"
returnsINFO
whenDD_LOG_LEVEL
is an empty stringos.environ.get("DD_LOG_LEVEL", "INFO")
returns an empty string whenDD_LOG_LEVEL
is an empty string
On further review I prefer your suggestion because it does result in the warning to show that an empty string is an invalid log level.
WARNING tests.test_logger:logger.py:25 Invalid log level: Defaulting to INFO
I also added a test case to cover when DD_LOG_LEVEL
is not set.
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.
Regarding the test runner workflow - I'm planning on working on that in a separate PR in the interest of getting this initial package release completed.
What does this PR do?
Motivation
Start the Datadog Serverless Compatibility Layer in Python Azure Functions and Google Cloud Run Functions (1st gen).
https://datadoghq.atlassian.net/browse/SVLS-6074
Additional Notes
Describe how to test/QA your changes
See README
Tested in the following environments: