infrastructure/blender-buildbot-www
1
0
Fork 2
Code Issues 4 Pull Requests 1 Projects Releases Wiki Activity

Buildbot: Move checksum to JSON #7

Merged
Bart van der Braak merged 2 commits from move-checksum-to-json into develop 2024-07-16 10:09:54 +02:00
Conversation 6 Commits 2 Files Changed 3 +25 -6
Bart van der Braak commented 2024-07-15 14:56:24 +02:00
Owner
Copy Link

A new version of the build directory listing JSON endpoint that skips listing the .sha256 files and instead adds a checksum key-value pair to each entry.

This pull request also introduces a Dockerfile and Docker compose file that makes local development trivial (docker compose up -d --build).

Initial issue: infrastructure/blender-projects-platform#90

A new version of the build directory listing JSON endpoint that skips listing the `.sha256` files and instead adds a checksum key-value pair to each entry. ~~This pull request also introduces a Dockerfile and Docker compose file that makes local development trivial (`docker compose up -d --build`).~~ Initial issue: infrastructure/blender-projects-platform#90
Bart van der Braak added 2 commits 2024-07-15 14:56:25 +02:00
Sergey Sharybin was assigned by Bart van der Braak 2024-07-15 14:56:48 +02:00
Sergey Sharybin was unassigned by Bart van der Braak 2024-07-15 14:56:54 +02:00
Bart van der Braak requested review from Pablo Vazquez 2024-07-15 14:57:05 +02:00
Bart van der Braak requested review from Sergey Sharybin 2024-07-15 14:57:05 +02:00
Bart van der Braak commented 2024-07-15 15:00:54 +02:00
Author
Owner
Copy Link

@pablovazquez I know you develop relatively frequently within this repo, do these Docker changes conflict in any way with your workflow?

Also, @fsiddi at some point added instructions for using a Dockerfile but also added it to the .gitignore and I'm not entirely sure what the reason for that is.

@pablovazquez I know you develop relatively frequently within this repo, do these Docker changes conflict in any way with your workflow? Also, @fsiddi at some point added instructions for using a `Dockerfile` but also [added it to the `.gitignore`](https://projects.blender.org/infrastructure/blender-buildbot-www/commit/ee2b8dd29f5eb73d7d682dd3f720b92b0ac76a86) and I'm not entirely sure what the reason for that is.
👍 1
Sergey Sharybin reviewed 2024-07-15 15:56:21 +02:00
source/Build.php Outdated
@ -86,0 +90,4 @@
$this->sha256_checksum = trim(str_replace(["\r", "\n", "\t"], '', file_get_contents($sha256_file)));
} else {
// Compute SHA256 checksum.
$this->sha256_checksum = strtoupper(hash_file('sha256', $file_path));
Sergey Sharybin commented 2024-07-15 15:54:23 +02:00
Owner
Copy Link

Computing hash of files is not cheap, is not something we should be doing for an end-point which is considered fast.
If the checksum file is missing, it is an indication of some bigger problem, so might as well keep the field empty.

Computing hash of files is not cheap, is not something we should be doing for an end-point which is considered fast. If the checksum file is missing, it is an indication of some bigger problem, so might as well keep the field empty.
bartvdbraak marked this conversation as resolved
source/BuildsRenderer.php Outdated
@ -32,0 +32,4 @@
if ($version == 2) {
// If version 2 is requested, skip any .sha256 files and add a checksum key-value entry
if (substr($build->file_name, -7) === '.sha256') {
Sergey Sharybin commented 2024-07-15 15:56:00 +02:00
Owner
Copy Link

We have function endsWith(string $haystack, string $needle) utility function, so can simply do if (endsWith($build->file_name, '.sha256')).

We have `function endsWith(string $haystack, string $needle)` utility function, so can simply do `if (endsWith($build->file_name, '.sha256'))`.
bartvdbraak marked this conversation as resolved
Sergey Sharybin commented 2024-07-15 15:58:17 +02:00
Owner
Copy Link

The PR should target the develop branch. master is the production.
One day would be cool to unity it with other web projects and make more clear, but until then it is what it is.

The PR should target the `develop` branch. `master` is the `production`. One day would be cool to unity it with other web projects and make more clear, but until then it is what it is.
👍 2
Bart van der Braak added 1 commit 2024-07-15 18:21:51 +02:00
Bart van der Braak force-pushed move-checksum-to-json from 36e3ccdadb to 9551fd55cb 2024-07-15 18:22:48 +02:00 Compare
Bart van der Braak force-pushed move-checksum-to-json from 9551fd55cb to a4a0900193 2024-07-15 18:23:23 +02:00 Compare
Bart van der Braak force-pushed move-checksum-to-json from a4a0900193 to 95cc2e7286 2024-07-15 18:24:43 +02:00 Compare
Pablo Vazquez commented 2024-07-15 18:37:28 +02:00
Owner
Copy Link

@pablovazquez I know you develop relatively frequently within this repo, do these Docker changes conflict in any way with your workflow?

They don't, I always followed the instructions anyway so it's just a matter of removing my local Dockerfile. Thanks for asking!

> @pablovazquez I know you develop relatively frequently within this repo, do these Docker changes conflict in any way with your workflow? They don't, I always followed the instructions anyway so it's just a matter of removing my local Dockerfile. Thanks for asking!
👍 1
Bart van der Braak changed target branch from master to develop 2024-07-15 18:42:25 +02:00
Francesco Siddi commented 2024-07-16 01:56:02 +02:00
Owner
Copy Link

Also, @fsiddi at some point added instructions for using a Dockerfile but also added it to the .gitignore and I'm not entirely sure what the reason for that is.

Sergey did not accept having a Dockerfile in the repo at the time. Check with him if that's acceptable now. If so, add the Dockerfile in a separate commit.

> Also, @fsiddi at some point added instructions for using a `Dockerfile` but also [added it to the `.gitignore`](https://projects.blender.org/infrastructure/blender-buildbot-www/commit/ee2b8dd29f5eb73d7d682dd3f720b92b0ac76a86) and I'm not entirely sure what the reason for that is. Sergey did not accept having a Dockerfile in the repo at the time. Check with him if that's acceptable now. If so, add the Dockerfile in a separate commit.
👍 1
Sergey Sharybin reviewed 2024-07-16 09:56:53 +02:00
source/Build.php Outdated
@ -22,6 +22,7 @@ class Build {
public $file_size;
public $file_extension;
public $release_cycle;
public $sha256_checksum; // New property
Sergey Sharybin commented 2024-07-16 09:56:11 +02:00
Owner
Copy Link

Don't refer to a time-line type of a thing. The property is new from the perspective of this PR, but once it lands the property is not new, is kist there. Simplty public $sha256_checksum;.

Don't refer to a time-line type of a thing. The property is new from the perspective of this PR, but once it lands the property is not new, is kist there. Simplty `public $sha256_checksum;`.
bartvdbraak marked this conversation as resolved
Sergey Sharybin commented 2024-07-16 09:58:50 +02:00
Owner
Copy Link

I suggest splitting Docker to a separate PR, so then we can review it separate and make a decision in the updated realities of who and how developers this site.

I suggest splitting Docker to a separate PR, so then we can review it separate and make a decision in the updated realities of who and how developers this site.
👍 1
Bart van der Braak force-pushed move-checksum-to-json from 95cc2e7286 to 54cf8534f4 2024-07-16 10:00:33 +02:00 Compare
Bart van der Braak force-pushed move-checksum-to-json from 54cf8534f4 to 2a2868fd1b 2024-07-16 10:03:25 +02:00 Compare
Bart van der Braak force-pushed move-checksum-to-json from 2a2868fd1b to 0ac2ca0896 2024-07-16 10:05:45 +02:00 Compare
Bart van der Braak commented 2024-07-16 10:08:25 +02:00
Author
Owner
Copy Link

Moved any Docker-related changes to #8.

Moved any Docker-related changes to #8.
Bart van der Braak merged commit c7219a3595 into develop 2024-07-16 10:09:54 +02:00
Bart van der Braak deleted branch move-checksum-to-json 2024-07-16 10:09:54 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
Pablo Vazquez
Sergey Sharybin
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
4 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: infrastructure/blender-buildbot-www#7
No description provided.
Delete Branch "move-checksum-to-json"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?