Skip to content

Conversation

@matusdrobuliak66
Copy link
Collaborator

@matusdrobuliak66 matusdrobuliak66 commented Sep 9, 2024

What do these changes do?

  • 🐛 fix rabbit mq unique queue name

Related issue/s

How to test

Dev-ops checklist

@matusdrobuliak66 matusdrobuliak66 marked this pull request as ready for review September 9, 2024 09:50
@matusdrobuliak66 matusdrobuliak66 self-assigned this Sep 9, 2024
@matusdrobuliak66 matusdrobuliak66 added this to the Eisbock milestone Sep 9, 2024
@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 9, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
1 Security Hotspot

See analysis details on SonarCloud

@codecov
Copy link

codecov bot commented Sep 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.5%. Comparing base (cafbf96) to head (fc3b616).
Report is 541 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #6328      +/-   ##
=========================================
+ Coverage    84.5%   88.5%    +3.9%     
=========================================
  Files          10     962     +952     
  Lines         214   43819   +43605     
  Branches       25     260     +235     
=========================================
+ Hits          181   38801   +38620     
- Misses         23    4960    +4937     
- Partials       10      58      +48     
Flag Coverage Δ
integrationtests 63.5% <50.0%> (?)
unittests 85.8% <100.0%> (+1.2%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...tifications/_rabbitmq_exclusive_queue_consumers.py 66.1% <100.0%> (ø)

... and 971 files with indirect coverage changes

RABBIT_QUEUE_MESSAGE_DEFAULT_TTL_MS: Final[int] = 15 * _MINUTE * 1000

CHARACTERS = string.ascii_letters + string.digits
_GENERATE_RANDOM_STRING_LENGTH = 6
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@pcrespov I guess I can create a function for this and add it to utils_secrets.py? we do not yet have one that combines letters and numbers as far as I can see.

Copy link
Member

Choose a reason for hiding this comment

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

I guess here generate_passcode might work for this case as well.

In any case, more than a secure here the idea is to generate some sort of unique identifer.
I would start a new module with helpesr like servicelib.identifiers_utils and add functions that can produce unique identifiers e.g. based on some unique feature (time, hostname, etc...)

import hashlib
import time
from models_library.basic_types import IdStr

def short_sha256(input_string, length=8) -> IdStr:
    sha_signature = hashlib.sha256(input_string.encode()).hexdigest()
    return IdStr(sha_signature[:length])

# Example usage
def get_rabbitma_client_unique_name(prefix: str)
   hostname = socket.gethostname()
   unique_id = short_sha256( f"{time.time()}" + hostname , length=8)
   return f"{prefix}_{hostname}_{unique_id}"

NOTE that the value you pass to input_string provides the "uniqueness" scope of that value. Therefore you use as discriminator.

Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

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

see my comment. thanks! And let's see what happens.

Copy link
Member

@pcrespov pcrespov left a comment

Choose a reason for hiding this comment

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

Thx. Left some comments for your consideration.

Let's check after they merge whether they restart faster. Add the 🚨

RABBIT_QUEUE_MESSAGE_DEFAULT_TTL_MS: Final[int] = 15 * _MINUTE * 1000

CHARACTERS = string.ascii_letters + string.digits
_GENERATE_RANDOM_STRING_LENGTH = 6
Copy link
Member

Choose a reason for hiding this comment

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

I guess here generate_passcode might work for this case as well.

In any case, more than a secure here the idea is to generate some sort of unique identifer.
I would start a new module with helpesr like servicelib.identifiers_utils and add functions that can produce unique identifiers e.g. based on some unique feature (time, hostname, etc...)

import hashlib
import time
from models_library.basic_types import IdStr

def short_sha256(input_string, length=8) -> IdStr:
    sha_signature = hashlib.sha256(input_string.encode()).hexdigest()
    return IdStr(sha_signature[:length])

# Example usage
def get_rabbitma_client_unique_name(prefix: str)
   hostname = socket.gethostname()
   unique_id = short_sha256( f"{time.time()}" + hostname , length=8)
   return f"{prefix}_{hostname}_{unique_id}"

NOTE that the value you pass to input_string provides the "uniqueness" scope of that value. Therefore you use as discriminator.

random_string = "".join(
random.choice(CHARACTERS) for _ in range(_GENERATE_RANDOM_STRING_LENGTH)
)
return f"{base_name}_{socket.gethostname()}_{random_string}"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not convinced by this. The whole idea of using the hostname here was to make it unique.

I would rather drop the hostname and use a larger random string, which makes collisions less likely. You might even get away with using a uuid from which you strip the -.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please note that all your tests will fail if something fishy is happening with the queue names. You might want to check what your change introduced.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants