Added support for cs3 api #72
Conversation
dynamic-entropy
force-pushed
the
cs3api-dev
branch
from
bce7506
to
49e84bb
Compare
There was a problem hiding this comment.
Looks all right to me for how I understand GFAL and Davix, of course the final word to @mpatrascoiu ;-)
| //Assuming urls with +3rd prefix have been deprecated | ||
|
|
||
| std::string src_url, dst_url, token; | ||
|
|
There was a problem hiding this comment.
Maybe add a comment referring the code mentioned in cern-fts/gfal2#7 (comment)
| } | ||
| } | ||
|
|
||
| void Credentials::addCredentials(std::string uri, std::string token, bool token_write_access){ |
There was a problem hiding this comment.
token can be mode const& reference.
There was a problem hiding this comment.
Regarding token_write_access, I found it a bit difficult to convert from this role to isDst role, which is the flag's corresponding purpose in the Credential object.
There was a problem hiding this comment.
Earlier we misunderstood the implications of token_write_access. We were trying to add an extra var. I missed to correct it here. Changed it back to token_write_access.
|
|
||
| //In pull mode original uri = destination uri | ||
| if(_params.getCopyMode() == Davix::CopyMode::Pull){ | ||
| for (reva::CredentialMap::iterator itr = cmap.begin(); itr != cmap.end(); ++itr) { |
There was a problem hiding this comment.
Is this an iteration over the whole map?
Wouldn't that mean the active_token and passive_token values depend only on the last element in the map?
Shouldn't there be a filtering by uri?
There was a problem hiding this comment.
This credMap is supposed to live only in the context of that transfer. So we only have two token at any time.
If this is not destroyed when that transfer job completes then we would have to find an alternative.
There was a problem hiding this comment.
I understand. If the map is guaranteed to only have 2 elements in it, it's ok.
Making sure I have to have the correct rationale:
active_token/passive_tokennaming is influenced primarily by 3rd push/pull role (E.g.: destination is active for 3rd pull / source is active for 3rd push)
There was a problem hiding this comment.
Yes, that is correct. The names are borrowed from a draft RFC on HTTP-TPC.
- Added cs3s to httpizeProtocol - corrected isDst to token_wtite_access
| @@ -49,7 +49,7 @@ class DAVIX_EXPORT Credentials { | |||
| class DAVIX_EXPORT CredentialProvider{ | |||
| public: | |||
| CredentialProvider(){}; | |||
| void updateCredentials(Credentials &creds, std::string uri, bool token_write_access); | |||
| static void updateCredentials(Credentials &creds, std::string uri, bool token_write_access); | |||
There was a problem hiding this comment.
Hello,
On second thought, to keep this consistent with the gcloud::CredentialProvider and the way it is used in Gfal2, let's keep reva::updateCredentials(..) without static in it.
|
|
||
| //In pull mode original uri = destination uri | ||
| if(_params.getCopyMode() == Davix::CopyMode::Pull){ | ||
| for (reva::CredentialMap::iterator itr = cmap.begin(); itr != cmap.end(); ++itr) { |
There was a problem hiding this comment.
I understand. If the map is guaranteed to only have 2 elements in it, it's ok.
Making sure I have to have the correct rationale:
active_token/passive_tokennaming is influenced primarily by 3rd push/pull role (E.g.: destination is active for 3rd pull / source is active for 3rd push)
|
Many thanks for the contributions! |
This PR extends
libdavixwithCS3 Request Protocolinterface and corresponding Authorisation, Authentication and Transfer Request parameters, and methods to interact with them as required by PR cern-fts/gfal2#7.A transfer between a ScienceMesh site and a Grid Site introduces a possibility for FTS to Authenticate with two different tokens for certain transfers. And these identities should be provided by the user on whose behalf the transfers are made.
Later Transfer Headers are determined based on role (active, passive) of endpoints and mode ( push, pull, streamed) of transfer.