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

test_datalad_credential_helper - AttributeError: 'NoneType' object has no attribute 'credential' #7347

Closed
yarikoptic opened this issue Mar 23, 2023 · 8 comments · Fixed by #7357
Assignees
Labels
spurious-test-failure Tests that fail unreliably

Comments

@yarikoptic
Copy link
Member

@adswa summarized observing it on #7340 (comment)

I have ran into it on a fresh (woohoo -- a new laptop) box in a sample run of pytest -n10 -v datalad (done in 2 minutes)

details
Traceback (most recent call last):
  File "/home/yoh/datalad/datalad/cmd.py", line 537, in close
    for source, data in self.generator:
  File "<frozen _collections_abc>", line 330, in __next__
  File "/home/yoh/datalad/datalad/runner/nonasyncrunner.py", line 97, in send
    return self._locked_send(message)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/yoh/datalad/datalad/runner/nonasyncrunner.py", line 122, in _locked_send
    self._check_result()
  File "/home/yoh/datalad/datalad/runner/nonasyncrunner.py", line 93, in _check_result
    self.runner._check_result()
  File "/home/yoh/datalad/datalad/runner/nonasyncrunner.py", line 295, in _check_result
    raise CommandError(
datalad.runner.exception.CommandError: CommandError: '/home/yoh/venvs/dev3.11/bin/python -i -u -q -' failed with exitcode 1

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/lib/python3.11/logging/__init__.py", line 1113, in emit
    stream.write(msg + self.terminator)
ValueError: I/O operation on closed file.
Call stack:
  File "/home/yoh/datalad/datalad/cmd.py", line 209, in __del__
    self.close()
  File "/home/yoh/datalad/datalad/cmd.py", line 551, in close
    lgr.error(
Message: '%s subprocess exited with %s (%s)'
Arguments: (BatchedCommand(command=<<['/home/yoh/ve++43 chars++'-']>>, encoding='UTF-8', exception_on_timeout=True, generator=<_ResultGenerator>, last_request="import time; print('a')", output_proc=None, path=None, return_code=None, runner=<WitlessRunner>, stderr_output=b'>>> >>> ', stdin_queue=<Queue>, timeout=0.5, wait_timed_out=None), '1', CommandError(''))


=========================================================== FAILURES ============================================================
________________________________________________ test_datalad_credential_helper _________________________________________________
[gw0] linux -- Python 3.11.2 /home/yoh/venvs/dev3.11/bin/python

path = '/tmp/datalad_temp_test_datalad_credential_helperyaookelr'

    @with_tempfile
    def test_datalad_credential_helper(path=None):

        ds = Dataset(path).create()

        # tell git to use git-credential-datalad
        ds.config.add('credential.helper', 'datalad', scope='local')
        ds.config.add('datalad.credentials.githelper.noninteractive', 'true',
                      scope='global')

        from datalad.downloaders.providers import Providers

        url1 = "https://datalad-test.org/some"
        url2 = "https://datalad-test.org/other"
        provider_name = "datalad-test.org"

        # `Providers` code is old and only considers a dataset root based on PWD
        # for config lookup. contextmanager below can be removed once the
        # provider/credential system is redesigned.
        with chpwd(ds.path):

            gitcred = GitCredentialInterface(url=url1, repo=ds)

            # There's nothing set up yet, helper should return empty
            gitcred.fill()
            eq_(gitcred['username'], '')
            eq_(gitcred['password'], '')

            # store new credentials
            # Note, that `Providers.enter_new()` currently uses user-level config
            # files for storage only. TODO: make that an option!
            # To not mess with existing ones, fail if it already exists:

            cfg_file = Path(Providers._get_providers_dirs()['user']) \
                       / f"{provider_name}.cfg"
            assert_false(cfg_file.exists())

            # Make sure we clean up
            from datalad.tests import _TEMP_PATHS_GENERATED
            _TEMP_PATHS_GENERATED.append(str(cfg_file))

            # Give credentials to git and ask it to store them:
            gitcred = GitCredentialInterface(url=url1, username="dl-user",
                                             password="dl-pwd", repo=ds)
            gitcred.approve()

            assert_true(cfg_file.exists())
            providers = Providers.from_config_files()
            p1 = providers.get_provider(url=url1, only_nondefault=True)
>           assert_is_instance(p1.credential, UserPassword)
E           AttributeError: 'NoneType' object has no attribute 'credential'

datalad/local/tests/test_gitcredential.py:122: AttributeError
----------------------------------------------------- Captured stdout call ------------------------------------------------------
create(ok): . (dataset)
======================================================= warnings summary ========================================================

previously we had possibly related #7165 which was addressed by #7209

WTF on that box (shows that I do not even have git user configured)

It is highly recommended to configure Git before using DataLad. Set both 'user.name' and 'user.email' configuration variables.

WTF

configuration <SENSITIVE, report disabled by configuration>

credentials

  • keyring:
    • active_backends:
      • PlaintextKeyring with no encyption v.1.0 at /home/yoh/.local/share/python_keyring/keyring_pass.cfg
    • config_file: /home/yoh/.config/python_keyring/keyringrc.cfg
    • data_root: /home/yoh/.local/share/python_keyring

datalad

  • version: 0.18.2+58.ge22e73f99

dataset

  • branches:
    • maint@e22e73f
  • id: None
  • path: /home/yoh/datalad
  • repo: GitRepo

dependencies

  • annexremote: 1.6.0
  • boto: 2.49.0
  • cmd:7z: 16.02
  • cmd:annex: 10.20230126
  • cmd:bundled-git: UNKNOWN
  • cmd:git: 2.39.2
  • cmd:ssh: 9.2p1
  • cmd:system-git: 2.39.2
  • cmd:system-ssh: 9.2p1
  • humanize: 4.6.0
  • iso8601: 1.1.0
  • keyring: 23.13.1
  • keyrings.alt: 4.2.0
  • msgpack: 1.0.5
  • platformdirs: 3.1.1
  • requests: 2.28.2

environment

  • LANG: en_US.UTF-8
  • PATH: /home/yoh/venvs/dev3.11/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games

extensions

git-annex

  • build flags:
    • Assistant
    • Webapp
    • Pairing
    • Inotify
    • DBus
    • DesktopNotify
    • TorrentParser
    • MagicMime
    • Benchmark
    • Feeds
    • Testsuite
    • S3
    • WebDAV
  • dependency versions:
    • aws-0.22.1
    • bloomfilter-2.0.1.0
    • cryptonite-0.29
    • DAV-1.3.4
    • feed-1.3.2.1
    • ghc-9.0.2
    • http-client-0.7.13.1
    • persistent-sqlite-2.13.1.0
    • torrent-10000.1.1
    • uuid-1.3.15
    • yesod-1.6.2.1
  • key/value backends:
    • SHA256E
    • SHA256
    • SHA512E
    • SHA512
    • SHA224E
    • SHA224
    • SHA384E
    • SHA384
    • SHA3_256E
    • SHA3_256
    • SHA3_512E
    • SHA3_512
    • SHA3_224E
    • SHA3_224
    • SHA3_384E
    • SHA3_384
    • SKEIN256E
    • SKEIN256
    • SKEIN512E
    • SKEIN512
    • BLAKE2B256E
    • BLAKE2B256
    • BLAKE2B512E
    • BLAKE2B512
    • BLAKE2B160E
    • BLAKE2B160
    • BLAKE2B224E
    • BLAKE2B224
    • BLAKE2B384E
    • BLAKE2B384
    • BLAKE2BP512E
    • BLAKE2BP512
    • BLAKE2S256E
    • BLAKE2S256
    • BLAKE2S160E
    • BLAKE2S160
    • BLAKE2S224E
    • BLAKE2S224
    • BLAKE2SP256E
    • BLAKE2SP256
    • BLAKE2SP224E
    • BLAKE2SP224
    • SHA1E
    • SHA1
    • MD5E
    • MD5
    • WORM
    • URL
    • X*
  • operating system: linux x86_64
  • remote types:
    • git
    • gcrypt
    • p2p
    • S3
    • bup
    • directory
    • rsync
    • web
    • bittorrent
    • webdav
    • adb
    • tahoe
    • glacier
    • ddar
    • git-lfs
    • httpalso
    • borg
    • hook
    • external
  • supported repository versions:
    • 8
    • 9
    • 10
  • upgrade supported from repository versions:
    • 0
    • 1
    • 2
    • 3
    • 4
    • 5
    • 6
    • 7
    • 8
    • 9
    • 10
  • version: 10.20230126

location

  • path: /home/yoh/datalad
  • type: dataset

python

  • implementation: CPython
  • version: 3.11.2

system

  • distribution: debian/n/a/bookworm
  • encoding:
    • default: utf-8
    • filesystem: utf-8
    • locale.prefered: UTF-8
  • filesystem:
    • CWD:
      • max_pathlength: 4096
      • mount_opts: rw,noatime,compress=lzo,ssd,space_cache=v2,subvolid=257,subvol=/@home
      • path: /home/yoh/datalad
      • type: btrfs
    • HOME:
      • max_pathlength: 4096
      • mount_opts: rw,noatime,compress=lzo,ssd,space_cache=v2,subvolid=257,subvol=/@home
      • path: /home/yoh
      • type: btrfs
    • TMP:
      • max_pathlength: 4096
      • mount_opts: rw,noatime,compress=lzo,ssd,space_cache=v2,subvolid=256,subvol=/@rootfs
      • path: /tmp
      • type: btrfs
  • max_path_length: 273
  • name: Linux
  • release: 6.1.0-6-amd64
  • type: posix
  • version: Facilitate space-efficient throw-away clones of dataset handles #1 SMP PREEMPT_DYNAMIC Debian 6.1.15-1 (2023-03-05)
but an annoying part is that rerunning the failed tests -- ie this only one -- passes it after!
(dev3.11) yoh@bilena:~/datalad$ time python -m pytest -n 10 -v --lf datalad
================================================================ test session starts =================================================================
platform linux -- Python 3.11.2, pytest-7.2.2, pluggy-1.0.0 -- /home/yoh/venvs/dev3.11/bin/python
cachedir: .pytest_cache
rootdir: /home/yoh/datalad, configfile: tox.ini
plugins: xdist-3.2.1, fail-slow-0.3.0, cov-4.0.0
[gw0] linux Python 3.11.2 cwd: /home/yoh/datalad                             
[gw1] linux Python 3.11.2 cwd: /home/yoh/datalad                             
[gw2] linux Python 3.11.2 cwd: /home/yoh/datalad                             
[gw3] linux Python 3.11.2 cwd: /home/yoh/datalad                             
[gw4] linux Python 3.11.2 cwd: /home/yoh/datalad                             
[gw5] linux Python 3.11.2 cwd: /home/yoh/datalad                             
[gw6] linux Python 3.11.2 cwd: /home/yoh/datalad                             
[gw7] linux Python 3.11.2 cwd: /home/yoh/datalad                             
[gw8] linux Python 3.11.2 cwd: /home/yoh/datalad                             
[gw9] linux Python 3.11.2 cwd: /home/yoh/datalad                             
[gw0] Python 3.11.2 (main, Feb 12 2023, 00:48:52) [GCC 12.2.0]               
[gw1] Python 3.11.2 (main, Feb 12 2023, 00:48:52) [GCC 12.2.0]                
[gw2] Python 3.11.2 (main, Feb 12 2023, 00:48:52) [GCC 12.2.0]                 
[gw3] Python 3.11.2 (main, Feb 12 2023, 00:48:52) [GCC 12.2.0]                  
[gw4] Python 3.11.2 (main, Feb 12 2023, 00:48:52) [GCC 12.2.0]                   
[gw5] Python 3.11.2 (main, Feb 12 2023, 00:48:52) [GCC 12.2.0]                    
[gw6] Python 3.11.2 (main, Feb 12 2023, 00:48:52) [GCC 12.2.0]                     
[gw7] Python 3.11.2 (main, Feb 12 2023, 00:48:52) [GCC 12.2.0]                      
[gw9] Python 3.11.2 (main, Feb 12 2023, 00:48:52) [GCC 12.2.0]                       
[gw8] Python 3.11.2 (main, Feb 12 2023, 00:48:52) [GCC 12.2.0]                        
gw0 [1] / gw1 [1] / gw2 [1] / gw3 [1] / gw4 [1] / gw5 [1] / gw6 [1] / gw7 [1] / gw8 [1] / gw9 [1]
scheduling tests via LoadScheduling

datalad/local/tests/test_gitcredential.py::test_datalad_credential_helper
[gw0] [100%] PASSED datalad/local/tests/test_gitcredential.py::test_datalad_credential_helper

================================================================== warnings summary ==================================================================
../venvs/dev3.11/lib/python3.11/site-packages/requests_ftp/ftp.py:9: 10 warnings
  /home/yoh/venvs/dev3.11/lib/python3.11/site-packages/requests_ftp/ftp.py:9: DeprecationWarning: 'cgi' is deprecated and slated for removal in Python 3.13
    import cgi

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
=========================================================== 1 passed, 10 warnings in 2.32s ===========================================================
It is highly recommended to configure Git before using DataLad. Set both 'user.name' and 'user.email' configuration variables.

real    0m2.470s
user    0m6.951s
sys     0m0.810s

so likely it is some annoying interplay with having plain keyring just established or smth?

@yarikoptic yarikoptic added the spurious-test-failure Tests that fail unreliably label Mar 23, 2023
@yarikoptic
Copy link
Member Author

situation is more fun -- it fails again when I run all tests (in parallel) now paired with another failure

FAILED datalad/downloaders/tests/test_http.py::test_access_denied - AssertionError: shouldn't happen; question() failed
FAILED datalad/local/tests/test_gitcredential.py::test_datalad_credential_helper - AttributeError: 'NoneType' object has no attribute 'credential'

@adswa
Copy link
Member

adswa commented Mar 23, 2023

FAILED datalad/downloaders/tests/test_http.py::test_access_denied - AssertionError: shouldn't happen; question() failed

This rings a bell, I saw this when I looked into #6733 on Windows

@bpoldrack
Copy link
Member

Happened (pretty regularly actually) in #7344 as well, today

@yarikoptic
Copy link
Member Author

so it is some interaction with other tests. Unfortunately I have not got the ultimate minimal but here is one which does trigger it:

(dev3.11) yoh@bilena:~/datalad$ time python -m pytest -v -n 10 datalad/local/tests/  datalad/distributed/ datalad/cli datalad/support/ 
...
==================================================== short test summary info ====================================================
FAILED datalad/local/tests/test_gitcredential.py::test_datalad_credential_helper - AttributeError: 'NoneType' object has no attribute 'credential'
========================= 1 failed, 465 passed, 47 skipped, 1 xfailed, 21 warnings in 64.70s (0:01:04) ==========================
It is highly recommended to configure Git before using DataLad. Set both 'user.name' and 'user.email' configuration variables.

real	1m4.989s
user	3m44.290s
sys	1m25.728s

@bpoldrack bpoldrack self-assigned this Mar 27, 2023
@bpoldrack
Copy link
Member

Thx, @yarikoptic - reproduced. Looking into it. I suspect the lazy loading of providers gets in the way here. In any case, the delivered providers are not up-to-date.

@bpoldrack
Copy link
Member

bpoldrack commented Mar 27, 2023

Ok, so that is the problem indeed. Providers.from_config_files call get_dataset_root(""), which will always deliver the same "dataset root" (whatever that's supposed to mean in this context). And if it didn't change and the function isn't caled with reload, then it just delivers whatever providers it found before. Hence, the new provider config in the test isn't loaded.
While I don't understand what get_dataset_root("") is supposed to do under what circumstances, the only recent change I can see, is the type annotation to get_dataset_root. So, I am at the usual stage of "How did this ever work?"/"Why is it starting to fail only now?".

I suspect, that get_dataset_root("") was assumed to deliver the dataset root based on CWD, as in Path("") == Path("."). But something about that doesn't actually work.

@bpoldrack
Copy link
Member

bpoldrack commented Mar 27, 2023

I think, I got the problem. Not sure, why it only shows now, but since there's no recent change in the code involved, I suppose it's a change in tests.
The caching of Providers.from_config_files() doesn't actually work. That means, it will deliver the previous result, whenever PWD is in a dataset's root and was in a (sic!) dataset's root before. So, triggering is a matter of order in the tests. The test would pass if it's the first time in the python process, that provider configs are read while in a dataset root dir. The keyword here is: "a". The cache would not realize that PWD is in a different dataset's root now than it was before.

bpoldrack added a commit to bpoldrack/datalad that referenced this issue Mar 27, 2023
Providers.from_config_files() stores the result of
`get_dataset_root("")` and delivers the previous resut, if not called
with `reload` and the dataset root is the same.
However, what effectively is stored when CWD is a dataset's root is the
empty string - so, effectively `.`. But this would be the same for any
dataset root we are in, therefore this lazy loading would not recognize,
that the dataset changed. This lead to side-effects of the order of test
execution, when a test provided new config files. Those files would be
ignored, when providers were read from within a different dataset
before.

Hence, use the absolute path to store and compare.

Closes datalad#7347
@yarikoptic-gitmate
Copy link
Collaborator

Issue fixed in 0.18.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spurious-test-failure Tests that fail unreliably
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants