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

Use before assignment in downloadclient extractools #4493

Closed
rcarpa opened this issue Mar 25, 2021 · 5 comments
Closed

Use before assignment in downloadclient extractools #4493

rcarpa opened this issue Mar 25, 2021 · 5 comments

Comments

@rcarpa
Copy link
Contributor

rcarpa commented Mar 25, 2021

Motivation

[losanti@lxplus723 ~]$  rucio -v get mc16_13TeV:DAOD_FTAG4.24348858._000944.pool.root.1
2021-03-25 12:44:45,630	INFO	Processing 1 item(s) for input
2021-03-25 12:44:45,630	DEBUG	downloadclient.py	num_unmerged_items=1; num_dids=1; num_merged_items=1
2021-03-25 12:44:45,630	INFO	Getting sources of DIDs
2021-03-25 12:44:45,672	DEBUG	downloadclient.py	schemes: None
2021-03-25 12:44:45,672	DEBUG	downloadclient.py	rse_expression: None
2021-03-25 12:44:45,672	DEBUG	downloadclient.py	num DIDs for list_replicas call: 1
2021-03-25 12:44:45,768	DEBUG	downloadclient.py	num resolved files: 1
2021-03-25 12:44:45,782	DEBUG	downloadclient.py	"unzip -v" returned with exitcode 0
2021-03-25 12:44:45,802	DEBUG	rucio	Traceback (most recent call last):
  File "/cvmfs/atlas.cern.ch/repo/ATLASLocalRootBase/x86_64/rucio-clients/1.25.0.patch1/bin/rucio", line 132, in new_funct
    return function(*args, **kwargs)
  File "/cvmfs/atlas.cern.ch/repo/ATLASLocalRootBase/x86_64/rucio-clients/1.25.0.patch1/bin/rucio", line 1044, in download
    result = download_client.download_dids(items, args.ndownloader, trace_pattern)
  File "/cvmfs/atlas.cern.ch/repo/ATLASLocalRootBase/x86_64/rucio-clients/1.25.0.patch1/lib/python2.7/site-packages/rucio/client/downloadclient.py", line 288, in download_dids
    input_items = self._prepare_items_for_download(did_to_options, merged_items_with_sources, resolve_archives=resolve_archives)
  File "/cvmfs/atlas.cern.ch/repo/ATLASLocalRootBase/x86_64/rucio-clients/1.25.0.patch1/lib/python2.7/site-packages/rucio/client/downloadclient.py", line 1147, in _prepare_items_for_download
    self.extraction_tools = [tool for tool in self.extraction_tools if tool.is_useable()]
  File "/cvmfs/atlas.cern.ch/repo/ATLASLocalRootBase/x86_64/rucio-clients/1.25.0.patch1/lib/python2.7/site-packages/rucio/client/downloadclient.py", line 89, in is_useable
    self.logger(logging.DEBUG, 'Failed to execute: "%s"' % exitcode)
UnboundLocalError: local variable 'exitcode' referenced before assignment

Modification

The logger string should be constructed using the "cmd" variable

@bari12 bari12 closed this as completed in e920fa9 Mar 25, 2021
bari12 added a commit that referenced this issue Mar 25, 2021
…fore_assignment

Clients: fix use-before-assignment in download client. Fixes #4493
@bari12 bari12 added this to the 1.25.2-clients milestone Mar 25, 2021
@rodwalker
Copy link

This does not address the exception though right? It looks like 'tar --version' is the next cmd to be tested. I`m not sure how that can lead to an exception, but there is an increasing number of users hitting the problem.

Cheers,
Rod.

@rcarpa
Copy link
Contributor Author

rcarpa commented Mar 29, 2021

It mostly does. Rucio will continue to log an error message (which will also allow us to fix the real issue), but should not fail to download anything which doesn't need extraction.

The real issue may be related to the parallel test commit which changes some string handling (encode/decode), but we cannot see the root cause until this commit is deployed (it masks the real issue)

If you want to speed up things and users are tecky-enough, you can ask them to perform something like that

cd $TMPDIR
rsync -a "$RUCIO_HOME/" rucio
sed -i '89s/exitcode/cmd/' rucio/lib/python2.7/site-packages/rucio/client/downloadclient.py
PYTHONPATH="./rucio/lib/python2.7/site-packages:$PYTHONPATH" rucio/bin/rucio

which will allow us to see the real problem without waiting for the deployement of next rucio release.

@rodwalker
Copy link

Thanks, I was trying to do just that, and failing. I asked a user.
Cheers,
Rod.

@rodwalker
Copy link

User was fast

2021-03-29 11:15:18,901 DEBUG downloadclient.py Failed to execute: "tar --version"
2021-03-29 11:15:18,902 DEBUG downloadclient.py 'ascii' codec can't decode byte 0xc2 in position 29: ordinal not in range(128)

Do you see haw that can happen for some users/setup/shell and not others?

Cheers,
Rod.

@rcarpa
Copy link
Contributor Author

rcarpa commented Mar 29, 2021

Yes, it confirms that it's due to encode/decode change. Now it is even clear why it happens. Was a mystery to me before.
will work on the fix; but it is not needed for most users. The patch from the current issue will fix the problem for anybody who doesn't do extraction (in theory: for everybody)

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

No branches or pull requests

3 participants