Change current curl fork/exec with libdavix plugin for read operations #15130
Conversation
Support http/web protocols and implemented basic functions: open, read, position
1. X509_USER_PROXY
2. /tmp/x509up_u<(geteuid)>
3. X509_USER_{CERT|KEY}
If none of these is present, it will try to make connection without proxy/cert. X509_CERT_DIR can also be overwritten with environment variable.
0. Default davix log level; 1. DAVIX_LOG_WARNING; 2. DAVIX_LOG_VERBOSE; 3. DAVIX_LOG_DEBUG; 4. DAVIX_LOG_ALL; If provided value is not integer or it is not able to parse to int, it will use 0. Any other number from 4toN will be considered as 4. DAVIX_LOG_ALL which is most verbose;
It checks DavixErr if it was set or not also if FD was opened sucessfully and if return value is greater than 0. 0 - means it is end of file. If any of these are found it will raise an issue.
|
A new Pull Request was created by @juztas (Justas Balčas) for CMSSW_8_1_X. It involves the following packages: Utilities/DavixAdaptor The following packages do not have a category, yet: Utilities/DavixAdaptor @cmsbuild, @smuzaffar, @Dr15Jones, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are list here #13028 |
|
|
||
| IOSize DavixFile::write(const void *from, IOSize n) { | ||
| cms::Exception ex("FileWriteError"); | ||
| ex << "DavixFile::write(name='" << m_name << "') not implemented"; |
There was a problem hiding this comment.
Can you do ex.addContext("Calling DavixFile::write()"); here?
Do not use global variable for Davix Context Use unique_ptr in places where it is possible; Define m_name before throwing an error, otherwise it is null Check variables without NULL.
Throw exception if open is called on an open file
Compare bytes requested vs read and throw exceptions
Surround with braces {} as done in the rest of the code
|
Pull request #15130 was updated. @cmsbuild, @smuzaffar, @Dr15Jones, @davidlange6 can you please check and sign again. |
|
This is beginning to look good to me. Can you provide an overview of how you've tested this? Thanks. |
|
@juztas , can you please add few unit tests ? |
cmsbuild
added
tests-pending
comparison-pending
and removed
comparison-available
tests-approved
labels
|
Pull request #15130 was updated. @cmsbuild, @smuzaffar, @Dr15Jones, @davidlange6 can you please check and sign again. |
|
please test |
|
The tests are being triggered in jenkins. |
|
@bbockelm any further comments? I think I've finished. |
|
I'm happy! |
|
+1 |
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_8_1_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @smuzaffar |
cmsbuild
added
core-approved
fully-signed
and removed
core-pending
pending-signatures
labels
|
+1 |
|
@juztas , there are couple of unit test failing Can you please fix these? Looks like URL is not active any more. |
|
@smuzaffar I think it is more stable to use opendata.cern.ch as it also affected update to newest davix: cms-sw/cmsdist#2454 I am running some tests on this node (transfer-8), so it is better to stay away from it for now. |
http/https/web was tested by turning on http[s] access for an xrootd server. More tests are upcoming to identify the gain/loss.
This requires davix 0.6.3 (cms-sw/cmsdist#2355)