Repository navigation
Conversation
|
|
||
| if specific_file_restrictions == "restricted": | ||
| # https://cds.cern.ch/admin/webaccess/webaccessadmin.py/showroledetails?id_role=69 | ||
| groups.add("cern-personnel") |
There was a problem hiding this comment.
this should be handled better for future abstraction
There was a problem hiding this comment.
btw, this we settled needs to be changed to cern-accounts-primary (to be double checked)
There was a problem hiding this comment.
that was a self-note :) currently in the codebase we handle the "restricted" case only for HR.... I will update it in an upcoming PR because I want to get a better look in the code.
| elif specific_file_restrictions in group_mappings: | ||
| # last resort: a simple keyword mapped to CERN e-group(s) | ||
| groups.update(group_mappings[specific_file_restrictions]) | ||
| else: | ||
| raise ManualImportRequired( | ||
| message="Unexpected permission format.", | ||
| field="access", | ||
| subfield="subject.id", | ||
| stage="load", | ||
| recid=self.record.recid, | ||
| priority="critical", | ||
| value=specific_file_restrictions, | ||
| ) | ||
|
|
There was a problem hiding this comment.
here is new code to handle restrictions based on the CDS_ACCESS_MAPPING config or fail if unknown
| self.files = self.compute_files() | ||
| self.access = self.compute_access() | ||
| if self.representative_file is not None: | ||
| self.publication_date = arrow.get( |
There was a problem hiding this comment.
this was moved to the transform_versions file and passed down.
| # statuses — can't be represented, so hard-stop for manual review. | ||
| recid = str(file["recid"]) | ||
| distinct_statuses = {f["status"] for f in self.own_file_dumps} | ||
| if len(distinct_statuses) > 1: |
There was a problem hiding this comment.
Important addition: if we have any mixed file restriction e.g public/restricted or even restrictedA/restrictedB on the same version we fail
There was a problem hiding this comment.
In this case EP records will fail, because in the normal workflow it should separate the restricted and public files in the load, and we're failing in transform, see all my ep records for LHCf are failing
| { | ||
| "message": "Record has individual file restrictions", | ||
| "value": file["status"], | ||
| "value": status, |
There was a problem hiding this comment.
this is based now on the combined file statuses. We can handle now only one uniformed value.
29bc0a1 to
bdd3079
Compare
| # public and some restricted, and/or several distinct restriction | ||
| # statuses — can't be represented, so hard-stop for manual review. | ||
| recid = str(file["recid"]) | ||
| distinct_statuses = {f["status"] for f in self.own_file_dumps} |
There was a problem hiding this comment.
this line might be a bit unclear when reading, I think own_file_dumps might have been a bad name choice earlier during the refactoring
| # firerole, bare [CERN] e-group, ...) is RecordParent.resolve_grants()'s | ||
| # job at load time; it hard-raises on anything it can't resolve. Here we | ||
| # only decide public-vs-restricted and carry the raw status as `meta`. | ||
| status = distinct_statuses.pop() |
There was a problem hiding this comment.
what if there is no statuses? or status is empty string? how will it populate?
There was a problem hiding this comment.
Thanks, I will check that and add a test too
There was a problem hiding this comment.
Actually, it seems that all files have the status field and is `` if there is no value
There was a problem hiding this comment.
yes that is the case, but I am wondering what comes out in terms of grants. I guess there will be none created and file will be public?
| return { | ||
| "access_obj": {"record": record_access, "files": "restricted"}, | ||
| "meta": file["status"], | ||
| "meta": status, |
There was a problem hiding this comment.
will this pass original status from the legacy or massaged one? I am asking because we store this in the metadata at the end for clarity and it would be great if the metadata status on the file is preserved as it was in legacy
There was a problem hiding this comment.
At this level, the status comes from the file_dump from legacy.
bdd3079 to
cba9fbf
Compare
| elif any( | ||
| kw in specific_file_restrictions | ||
| for kw in ("firerole: allow group", "allow email") | ||
| ): | ||
| meta_str = specific_file_restrictions.replace("\r\n", "\n") | ||
|
|
||
| # Parse groups | ||
| group_matches = re.search(r'allow group\s+((?:"[^"]+",?\s*)+)', meta_str) | ||
| if group_matches: | ||
| group_values = re.findall(r'"([^"]+)"', group_matches.group(1)) | ||
| for g in group_values: | ||
| groups.add(self._normalize_group_name(g)) | ||
|
|
||
| # Parse emails | ||
| email_matches = re.search(r'allow email\s+((?:"[^"]+",?\s*)+)', meta_str) | ||
| if email_matches: | ||
| email_values = re.findall(r'"([^"]+)"', email_matches.group(1)) | ||
| emails.update(email_values) |
There was a problem hiding this comment.
maybe we should also raise if it doesnt match with group or email, since there is a restriction but it doesnt match
| def build(self): | ||
| """Populate ``files``/``access``/``publication_date``; return this version's dict.""" | ||
| """Populate ``files``/``access``; return this version's dict.""" | ||
| self.files = self.compute_files() |
There was a problem hiding this comment.
I also realized compute_access() only looks at each version’s new files, then copies earlier files into later versions without checking access.
So if v1 has a restricted file and v2 adds a new public file, v2 ends up with both files but marked as public
cba9fbf to
b682c75
Compare
3 participants
closes #627