Skip to content
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

Add cloud.aws.account_id config setting #1249

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

hmstepanek
Copy link
Contributor

@hmstepanek hmstepanek commented Nov 7, 2024

Overview

Add cloud.aws.account_id configuration setting. This is not used in this PR but will be used in future AWS linking for the user to provide the account_id when it cannot be derived by the instrumentation. See the Entity-Relationships agent spec.

@hmstepanek hmstepanek requested a review from a team as a code owner November 7, 2024 01:18
Copy link

github-actions bot commented Nov 7, 2024

🦙 MegaLinter status: ❌ ERROR

Descriptor Linter Files Fixed Errors Elapsed time
✅ PYTHON bandit 2 0 6.57s
✅ PYTHON black 3 1 0 2.31s
❌ PYTHON flake8 3 1 1.16s
✅ PYTHON isort 3 1 0 0.24s
✅ PYTHON pylint 3 0 8.27s

See detailed report in MegaLinter reports
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff

MegaLinter is graciously provided by OX Security

Comment on lines 79 to 88
if s and len(s) == 12:
try:
account_id = int(s)
return account_id
except ValueError:
pass
# Only log a warning if s is set.
if s:
_logger.warning(
"Improper configuration. cloud.aws.account_id = %s will be ignored because it is not a 12 digit number.", s
)
return None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if s and len(s) == 12:
try:
account_id = int(s)
return account_id
except ValueError:
pass
# Only log a warning if s is set.
if s:
_logger.warning(
"Improper configuration. cloud.aws.account_id = %s will be ignored because it is not a 12 digit number.", s
)
return None
if s and len(s) == 12 and s.isdigit():
return int(s)
# Only log a warning if s is set.
if s:
_logger.warning(
f"Improper configuration. cloud.aws.account_id = {s} will be ignored because it is not a 12 digit number."
)
return None

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isdigit looks cool! It looks like it also returns true if there is a power digit like ² which would result in a ValueError still being raised by the int cast though.

>>> s="01234²34"
>>> s.isdigit()
True
>>> int(s)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: invalid literal for int() with base 10: '01234²34'

Copy link
Contributor

@jairhenrique jairhenrique Nov 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hmstepanek you can use s.isdecimal() instead!

In [1]: s="01234²34"

In [2]: s.isdecimal()
Out[2]: False

In [3]: s="01234234"

In [4]: s.isdecimal()
Out[4]: True

@mergify mergify bot added the tests-failing label Nov 8, 2024
@codecov-commenter
Copy link

codecov-commenter commented Nov 8, 2024

Codecov Report

Attention: Patch coverage is 94.44444% with 1 line in your changes missing coverage. Please review.

Project coverage is 81.17%. Comparing base (f2cdd3e) to head (048e05b).

Files with missing lines Patch % Lines
newrelic/config.py 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1249      +/-   ##
==========================================
+ Coverage   81.15%   81.17%   +0.01%     
==========================================
  Files         200      200              
  Lines       21953    21971      +18     
  Branches     3479     3482       +3     
==========================================
+ Hits        17816    17834      +18     
- Misses       2986     2987       +1     
+ Partials     1151     1150       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants