Filling some gaps in the DTS manifest #78
Conversation
jeff-cohere
added
enhancement
|
jeff-cohere
force-pushed
the
metadata-endpoint
branch
from
8396b1c to
ba04b99
Compare
jeff-cohere
force-pushed
the
manifest-gaps
branch
from
5763a85 to
e1388d7
Compare
| foundOrcid := false | ||
| for _, pid := range userInfo.Idents { | ||
| if pid.Provider == "OrcID" { | ||
| foundOrcid = true | ||
| break | ||
| } | ||
| } | ||
| if !foundOrcid { | ||
| return userInfo, fmt.Errorf("KBase Auth2: No ORCID IDs associated with this user!") | ||
| } |
There was a problem hiding this comment.
is it worth setting the OrcID here, so that in the UserInfo function you don't do the same iteration through the list of Idents?
There was a problem hiding this comment.
I don't think it matters, unless a user has tens of thousands of ORCIDs associated(!). As it is now, kbaseUserInfo mirrors the information maintained by the KBase auth server, which doesn't prioritize the first ORCID. This mirrored structure is separate from our interpretation, which provides some clarity as to what we assume, which to me is more valuable than avoiding an extra loop.
| if pid.Provider == "OrcID" { | ||
| orcidIds = append(orcidIds, pid.UserName) | ||
| userInfo.Orcid = pid.UserName |
There was a problem hiding this comment.
is it guaranteed that there is only ever one Orcid associated with the user?
There was a problem hiding this comment.
No, but I don't know what we would do with more than one ORCID, so I'm assuming for now that we need to select one, and that the one we select is the first one.
There was a problem hiding this comment.
What happens if the user has different OrcIDs associated with their accounts on different platforms? (well, I know what would happen -- nothing. The more useful case to think about is where they have a bunch of OrcIDs assoc'd with their KBase account, one of which is associated with their account on another platform).
Not deliberately trying to be annoying here -- just thinking of the kinds of annoying situations that may pop up in the wild.
There was a problem hiding this comment.
Makes sense! I guess my logic is that I'd like to start simple, encounter real problems, and address accordingly. At least we have separate KBase and DTS user information so we can articulate our assumptions.
There was a problem hiding this comment.
I have been burned too many times by trying to keep it simple and assuming that KBase is doing sensible things...
There was a problem hiding this comment.
I believe it! Here's an issue: #81
This set of changes adds several fields to our frictionless DataPackage format, in service of providing information that is helpful for the destination database in processing payloads. It also gathers user metadata into a single simple type within the
authpackage and propagates the information for the user requesting a transfer into the transfer process.This should go in after #76.