Sitelet https://web.archive.org/web/20211107183801/https://github.com/python/cpython/pull/29247
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

Jump to bottom

bpo-45621: Small changes to mmap #29247

Merged
merged 7 commits into from Oct 29, 2021
Merged

bpo-45621: Small changes to mmap #29247

merged 7 commits into from Oct 29, 2021

Conversation

Reviewers

@eryksun eryksun

Assignees
No one assigned
Labels
CLA signed skip news
Projects
None yet
Milestone
No milestone
4 participants
@tjguk @eryksun @the-knights-who-say-ni @bedevere-bot
@tjguk
Copy link
Member

@tjguk tjguk commented Oct 27, 2021 •
edited by bedevere-bot
Loading

Following issue40915 a few small items still need to be addressed, mostly cosmetic / naming:

  • A comment should have been removed but wasn't
  • A comment should have been added but wasn't
  • The use of the string "TEST" should be avoided in tests particularly where there is a shared namespace and therefore the chance of a collision

https://bugs.python.org/issue45621
tjguk added 5 commits Oct 27, 2021
@tjguk
Remove unneeded include
10a060f
@tjguk
Remove verbose comment
45e50a0
@tjguk
Add a comment clarifying the ERROR_ALREADY_EXISTS situation
0e5d746
@tjguk
Use randomized names when testing tagged mmaps to avoid any risk of p…
8466ad5 
…arallel tests treading on each others' toes
@tjguk
Merge branch 'main' into issue45621-small-changes-to-mmap
b6969f5
eryksun
eryksun reviewed Oct 27, 2021
View changes
Lib/test/test_mmap.py Outdated
def random_tagname(length=10):
return "".join(random.choice(string.ascii_uppercase) for _ in range(length))
Copy link
Contributor

@eryksun eryksun Oct 27, 2021

How about including context in the name? For example:

tagname_prefix = f'python_{os.getpid()}_test_mmap'
def random_tagname(suffix_length=10):
    suffix = ''.join(random.choices(string.ascii_uppercase, k=suffix_length))
    return f'{tagname_prefix}_{suffix}'

Loading

Copy link
Member Author

@tjguk tjguk Oct 29, 2021

Ok, why not? (And thanks for the suggestion of random.choices: TIL)

Loading

tjguk added 2 commits Oct 29, 2021
@tjguk
Tie the random mmap tag name to the Python process
c481a0a
@tjguk
Merge branch 'main' into issue45621-small-changes-to-mmap
90f1d12
@tjguk tjguk merged commit 7bddd96 into python:main Oct 29, 2021
10 of 11 checks passed
Loading
@tjguk tjguk deleted the issue45621-small-changes-to-mmap branch Oct 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment