Skip to content

bpo-44771: rename namespacedata01 to namespacedata #27484

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

Closed
wants to merge 2 commits into from

Conversation

FFY00
Copy link
Member

@FFY00 FFY00 commented Jul 30, 2021

This should fix the buildbots, see
#27436 (comment)

Signed-off-by: Filipe Laíns [email protected]

https://bugs.python.org/issue44771

This should fix the buildbots, see
python#27436 (comment)

Signed-off-by: Filipe Laíns <[email protected]>
@FFY00 FFY00 requested review from jaraco and pablogsal July 30, 2021 14:55
@FFY00 FFY00 added skip news 🔨 test-with-buildbots Test PR w/ buildbots; report in status section labels Jul 30, 2021
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @FFY00 for commit bb6db97 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 30, 2021
@pablogsal
Copy link
Member

pablogsal commented Jul 30, 2021

There are some tests to update:

======================================================================
ERROR: test_files (test.test_importlib.test_reader.NamespaceReaderTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/dje/cpython-buildarea/pull_request.edelsohn-debian-z/build/Lib/test/test_importlib/test_reader.py", line 120, in test_files
    namespacedata = import_module('namespacedata01')
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/dje/cpython-buildarea/pull_request.edelsohn-debian-z/build/Lib/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen importlib._bootstrap>", line 1061, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1038, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1008, in _find_and_load_unlocked
ModuleNotFoundError: No module named 'namespacedata01'
======================================================================
ERROR: test_resource_path (test.test_importlib.test_reader.NamespaceReaderTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/dje/cpython-buildarea/pull_request.edelsohn-debian-z/build/Lib/test/test_importlib/test_reader.py", line 108, in test_resource_path
    namespacedata = import_module('namespacedata01')
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/dje/cpython-buildarea/pull_request.edelsohn-debian-z/build/Lib/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen importlib._bootstrap>", line 1061, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1038, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1008, in _find_and_load_unlocked
ModuleNotFoundError: No module named 'namespacedata01'
----------------------------------------------------------------------

Signed-off-by: Filipe Laíns <[email protected]>
@FFY00
Copy link
Member Author

FFY00 commented Jul 30, 2021

namespacedata01 is only used in the tests, so it shouldn't affect the frozen importlib. I just forgot the g at the end of my sed expression so it only replaced the first namespacedata01 in each line 😅

@FFY00 FFY00 added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 30, 2021
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @FFY00 for commit e1a911e 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 30, 2021
@pablogsal
Copy link
Member

pablogsal commented Jul 30, 2021

namespacedata01 is only used in the tests, so it shouldn't affect the frozen importlib.

Haha, yeah, I noticed when looking closer at the errors and then I updated the comment. Seems that not faster than you checked 😅

@jaraco
Copy link
Member

jaraco commented Jul 30, 2021

So is the plan here to just cause the files to be deleted on the buildbots, but not merge this change? Just force the buildbots to lose the files long enough that they start passing on main?

@FFY00
Copy link
Member Author

FFY00 commented Jul 30, 2021

Either that or plainly rename the data, but I just noticed I forgot .gitattributes 🤦

Let's let the buildbots run and after that see if they are still failing on new commits. I don't think they should since the data was removed here and they will have to check it out again.

@jaraco
Copy link
Member

jaraco commented Jul 30, 2021

I'd like not to have to rename the data, because that creates additional divergence from both ../data01 and also from the implementation in importlib_resources. I'd like to have whatever the permanent solution is not to carry the baggage of this incident.

@pablogsal
Copy link
Member

The original PR also introduced a file descriptor leak that needs to be addressed: https://buildbot.python.org/all/#/builders/320/builds/107

@jaraco
Copy link
Member

jaraco commented Jul 30, 2021

(moved comment to #27436)

1 similar comment
@jaraco
Copy link
Member

jaraco commented Jul 30, 2021

(moved comment to #27436)

@jaraco
Copy link
Member

jaraco commented Jul 31, 2021

This is done its duty and is no longer needed (see #27497). Thanks Filipe.

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