Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename {commit} to {build_commit}; {commit} now refers to triggering commit #64

Merged
merged 1 commit into from
May 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ A sample config file:

repo: datalad/datalad
vars:
path_prefix: '{year}//{month}//{day}/{ci}/{type}/{type_id}/{commit[:7]}'
path_prefix: '{year}//{month}//{day}/{ci}/{type}/{type_id}/{build_commit[:7]}'
ci:
github:
path: '{path_prefix}/{wf_name}/{number}/logs/'
Expand Down Expand Up @@ -295,8 +295,17 @@ Placeholder Definition
is the name of the branch to which the push was made (or
possibly the tag that was pushed, if using Appveyor); for
``release``, this is the name of the tag
``{commit}`` The hash of the commit the build ran against or that was
tagged for the release
``{build_commit}`` The hash of the commit the build ran against or that was
tagged for the release. Note that, for PR builds on
Travis and Appveyor, this is the hash of an autogenerated
merge commit.
``{commit}`` The hash of the original commit that triggered the build
or that was tagged for the release. For GitHub Actions
and all non-pull request builds, this is always the same
as ``{build_commit}``. For pull request builds on
Appveyor, this is the head of the PR branch. For pull
request builds on Travis, this is ``UNK``, as the API does
not expose the original commit.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but I still hope we workaround as discussed in #55 (comment), right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you mean pulling PR branches from GitHub, I consider that overkill.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but we do not have a better solution, do we?
The only other workaround I see is particular to DataLad setup since would require Appveyor run (now that we know that merge is done by github so the same across CIs) is to take information from Appveyor run which knows the commit for the build_commit and use for all CIs. Might also be needed/desired to establish persistent cache of knowing such mappings from build_commit to commit.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The best alternative I can think of is querying the GitHub API for information on the PR, checking whether the merge commit is the same as the build commit, and, if so, using the head commit listed there. It'll require some rewriting of the code that I'd prefer to do in a separate PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, thank you, let's proceed then

``{number}`` The run number of the workflow run (GitHub) or the build
number (Travis and Appveyor) [1]_
``{status}`` The success status of the workflow run (GitHub) or job
Expand Down
22 changes: 18 additions & 4 deletions src/tinuous/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,8 @@ class Asset(ABC, BaseModel):
created_at: datetime
event_type: EventType
event_id: str
commit: str
build_commit: str
commit: Optional[str]
number: int
status: str

Expand All @@ -151,6 +152,7 @@ class Config:

def path_fields(self) -> Dict[str, str]:
utc_date = self.created_at.astimezone(timezone.utc)
commit = "UNK" if self.commit is None else self.commit
return {
"year": utc_date.strftime("%Y"),
"month": utc_date.strftime("%m"),
Expand All @@ -160,7 +162,8 @@ def path_fields(self) -> Dict[str, str]:
"second": utc_date.strftime("%S"),
"type": self.event_type.value,
"type_id": self.event_id,
"commit": self.commit,
"build_commit": self.build_commit,
"commit": commit,
"number": str(self.number),
"status": self.status,
"common_status": COMMON_STATUS_MAP[self.status],
Expand Down Expand Up @@ -386,6 +389,7 @@ def from_workflow_run(
created_at=ensure_aware(run.created_at),
event_type=event_type,
event_id=event_id,
build_commit=run.head_sha,
commit=run.head_sha,
workflow_name=workflow.name,
workflow_file=workflow.path.split("/")[-1],
Expand Down Expand Up @@ -451,6 +455,7 @@ def from_workflow_run(
created_at=ensure_aware(run.created_at),
event_type=event_type,
event_id=event_id,
build_commit=run.head_sha,
commit=run.head_sha,
workflow_name=workflow.name,
workflow_file=workflow.path.split("/")[-1],
Expand Down Expand Up @@ -523,6 +528,7 @@ def path_fields(self) -> Dict[str, str]:
"ci": "github",
"type": "release",
"type_id": self.tag_name,
"build_commit": self.commit,
"commit": self.commit,
}

Expand Down Expand Up @@ -649,20 +655,25 @@ def from_job(
if event is None:
raise ValueError(f"Build has unknown event type {build['event_type']!r}")
event_id: str
commit: Optional[str]
if event is EventType.CRON:
event_id = created_at.strftime("%Y%m%dT%H%M%S")
commit = build["commit"]["sha"]
elif event is EventType.PUSH:
event_id = build["branch"]["name"]
commit = build["commit"]["sha"]
elif event is EventType.PULL_REQUEST:
event_id = str(build["pull_request_number"])
commit = None
else:
raise AssertionError(f"Unhandled EventType: {event!r}")
return cls(
client=client,
created_at=created_at,
event_type=event,
event_id=event_id,
commit=build["commit"]["sha"],
build_commit=build["commit"]["sha"],
commit=commit,
number=int(build["number"]),
job=removeprefix(job["number"], f"{build['number']}."),
job_id=job["id"],
Expand Down Expand Up @@ -783,15 +794,18 @@ def from_job(
if build.get("pullRequestId"):
event = EventType.PULL_REQUEST
event_id = build["pullRequestId"]
commit = build["pullRequestHeadCommitId"]
else:
event = EventType.PUSH
event_id = build["branch"]
commit = build["commitId"]
return cls(
client=client,
created_at=created_at,
event_type=event,
event_id=event_id,
commit=build["commitId"],
build_commit=build["commitId"],
commit=commit,
number=build["buildNumber"],
job=job["jobId"],
status=job["status"],
Expand Down