sbp commented on code in PR #1238:
URL:
https://github.com/apache/tooling-trusted-releases/pull/1238#discussion_r3229057062
##########
atr/api/__init__.py:
##########
@@ -857,6 +857,69 @@ async def policy_update(
).model_dump(mode="json"), 200
[email protected]
[email protected]
+@quart_schema.security_scheme([{"BearerAuth": []}])
+@quart_schema.validate_response(models.api.ProjectConfigResults, 200)
+async def project_config_upsert(
+ _project_config: Literal["project/config"],
+ data: models.api.ProjectConfigArgs,
+) -> DictResponse:
+ """
+ URL: POST /project/config
+
+ Upsert a project's full configuration.
+
+ Creates a project if it on by the specified key does not yet exist.
+ Otherwise, updates all specified fields for an existing project.
+ The caller must be a committee member of the specified committee,
+ and for an existing project committee_key must match the current value.
+ """
+ asf_uid = _jwt_asf_uid()
+ sections_applied: list[Literal["project", "policy"]] = []
+ created = False
+ async with db.session() as session:
+ existing = await session.project(key=str(data.project_key)).get()
+ if (existing is not None) and (existing.committee_key !=
str(data.committee_key)):
+ raise exceptions.BadRequest(
+ f"Project '{data.project_key}' belongs to committee
'{existing.committee_key}', not '{data.committee_key}'"
+ )
+ try:
+ async with storage.write_as_committee_member(str(data.committee_key),
asf_uid) as wacm:
+ if existing is None:
+ if (data.project is None) or (data.project.name is None) or
(not data.project.name.strip()):
+ raise exceptions.BadRequest(
+ f"Project '{data.project_key}' does not exist;
project.name is required to create it"
+ )
+ await wacm.project.create(
Review Comment:
This is calling `wacm` three times, but ideally it would be atomic. Perhaps
there could be another `wacm.project` method that does all three of these at
once, with a `begin_immediate`?
##########
atr/storage/writers/project.py:
##########
@@ -235,6 +237,53 @@ async def edit_metadata(self, form:
shared.projects.EditMetadataForm) -> None:
project_key=str(project.key),
)
+ async def edit_project_partial(
+ self,
+ project_key: safe.ProjectKey,
+ args: api.ProjectConfigProjectArgs,
+ ) -> None:
+ provided = args.model_fields_set
+ if not provided:
+ return
+ str_fields = {
Review Comment:
These are all `form.OptionalURL` in the web form
(`atr.shared.projects.EditMetadataForm`), which does validation. In the API,
these are just `str`, so I guess they're not validated the same way?
##########
atr/models/safe.py:
##########
@@ -24,11 +24,11 @@
import pydantic
-_ALPHANUM: Final = frozenset(string.ascii_letters + string.digits + "-")
+_ALPHANUM: Final = frozenset(string.ascii_letters + string.digits + "-+")
Review Comment:
This is used by committee key too, but I don't think we should allow `+` in
those.
##########
atr/datasources/apache.py:
##########
@@ -409,9 +439,12 @@ async def _update_committees(
committee.committers = project.members
# We create PMCs for now
committee.is_podling = False
- committee_info = committees_by_name.get(name)
+ whimsy_info = whimsy_committees_by_name.get(name)
+ if whimsy_info:
+ committee.name = whimsy_info.display_name
+ committee_info = commiteess.get(name)
Review Comment:
Typo: `commiteess`
##########
atr/models/api.py:
##########
@@ -377,11 +381,78 @@ def validate_policy_fields(self) -> "PolicyUpdateArgs":
return self
+class PolicyUpdateArgs(PolicyArgsBase):
+ project: safe.ProjectKey = schema.example("example")
+
+
class PolicyUpdateResults(schema.Strict):
endpoint: Literal["/policy/update"] = schema.alias("endpoint")
success: Literal[True] = schema.example(True)
+class ProjectConfigProjectArgs(schema.Strict):
+ name: str | None = None
+ description: str | None = None
+ short_description: str | None = None
+ homepage: str | None = None
+ lifecycle_page: str | None = None
+ download_page: str | None = None
+ bug_database: str | None = None
+ mailing_lists: str | None = None
+ repository: list[str] | None = None
+ standards: list[str] | None = None
+ # Changing any of these four reassigns existing releases to the cycle
+ # their version string now maps to.
+ version_method: sql.VersionMethod | None = None
+ version_pattern: str | None = None
+ cycle_match: str | None = None
+ branch_template: str | None = None
+
+ @pydantic.field_validator("version_method", mode="before")
+ @classmethod
+ def version_method_to_enum(cls, v):
+ if isinstance(v, str):
+ try:
+ return sql.VersionMethod.__members__[v]
Review Comment:
But for example we also have:
```python
@pydantic.field_validator("vote_mode", mode="before")
@classmethod
def vote_mode_to_enum(cls, v):
if isinstance(v, str):
try:
return sql.VoteMode(v)
except ValueError:
raise ValueError(f"'{v}' is not a valid VoteMode")
return v
```
And that's our pattern throughout, so I think this should return
`sql.VersionMethod(v)`, and use a `ValueError` exception branch.
##########
atr/api/__init__.py:
##########
@@ -857,6 +857,69 @@ async def policy_update(
).model_dump(mode="json"), 200
[email protected]
[email protected]
+@quart_schema.security_scheme([{"BearerAuth": []}])
+@quart_schema.validate_response(models.api.ProjectConfigResults, 200)
+async def project_config_upsert(
+ _project_config: Literal["project/config"],
+ data: models.api.ProjectConfigArgs,
+) -> DictResponse:
+ """
+ URL: POST /project/config
+
+ Upsert a project's full configuration.
+
+ Creates a project if it on by the specified key does not yet exist.
+ Otherwise, updates all specified fields for an existing project.
+ The caller must be a committee member of the specified committee,
+ and for an existing project committee_key must match the current value.
+ """
+ asf_uid = _jwt_asf_uid()
+ sections_applied: list[Literal["project", "policy"]] = []
+ created = False
+ async with db.session() as session:
Review Comment:
Maybe this existence and filtering stuff belong in the storage interface, in
the new wrapper function that I propose in the R894 comment.
##########
atr/datasources/apache.py:
##########
@@ -384,7 +411,10 @@ def _project_status(pmc: sql.Committee, project_key: str,
project_status: Projec
async def _update_committees(
- data: db.Session, ldap_projects: LDAPProjectsData, committees_by_name:
Mapping[str, Committee]
+ data: db.Session,
+ ldap_projects: LDAPProjectsData,
+ whimsy_committees_by_name: Mapping[str, WhimsyCommittee],
+ commiteess: dict[str, Committee],
Review Comment:
Typo: `commiteess`
##########
atr/datasources/apache.py:
##########
@@ -81,6 +82,20 @@ class User(schema.Strict):
class Committee(schema.Strict):
+ chair: str
Review Comment:
The commit message talks about removing a project chair, but I'm not sure
what that means. The only reference to a chair in the PR is this one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
