Skip to content

Conversation

@iwanders
Copy link
Contributor

@iwanders iwanders commented May 2, 2022

Hi everyone,

As my colleague @mikepurvis described in #1193, we are in the process of incorporating some Nix builds running on Hydra into our Jenkins pipelines. An important aspect of this is making sure that it is clear for developers where to find the logs for build failures, with our transition to Nix we should be able to provide this in a more specific manner than we previously could. However, we could not find any API endpoint that actually provides information about the build steps that make up a build.

I think the (currently undocumented) <build_id>/api/get-info endpoint was supposed to provide this information, so in this PR I extended that endpoint to provide information about all the build steps that make up a build. With this newly available information about the results of each individual build step, we will be able to link directly to compilation logs for failed steps.

Overview of changes:

  • Extended the <build_id/api/get-info endpoint to include build steps, and made it (mostly) a superset of the current /build/<build_id> api endpoint, this should allow for uniform handling between the two. The old fields are preserved for backwards compatibility.
  • Added a function to convert the build step object into Hash that can be returned through the JSON view.
  • Modified buildToHash to ensure the buildstatus and priority fields are always present.
  • Used yaml references in the hydra api to ensure fields identical between the build api endpoint and get-info stay in sync.
  • Rendered hydra API document.

This is the first time I'm using Perl, one of the things I couldn't quite figure out is how the current build api endpoint (so /build/<build_id) works. I reused this buildToHash function, but later noticed that this is not the function that provides that is used in the /build/<build_id> endpoint, as the build_status key was missing if the build was still ongoing. I think that endpoint is served by this as_json function. But I don't know enough Perl to figure out why we have two, or how I could call the latter to populate the results of the get-info endpoint as a starting point and then add the steps to that. It would be the ideal case if get-info is a superset of the simple /build/<build_id> api, currently it's close, but because there's two conversion functions it's not guaranteed. Ideally, these should be consolidated in the future to ensure the endpoints in the api are as consistent as possible.

Another thing that's worth considering is changing the name from /build/<build_id>/api/get-info to /build/<build_id>/get-info that would break backwards compatibility, but makes it more in line with the current api naming.

@ajs124
Copy link
Member

ajs124 commented Aug 11, 2022

Can you maybe write a test for this? I think this would probably fit into t/Hydra/Controller/API/checks.t.

Calling as_json sounds like it might be useful, yes. I'll try to look into that. As for why we have that and buildToHash, that's probably mainly historical reasons. Maybe @grahamc knows, since he only added ad_json a few months ago.

@iwanders
Copy link
Contributor Author

iwanders commented Aug 12, 2022

Thanks for taking a look at this @ajs124, appreciated!

Can you maybe write a test for this? I think this would probably fit into t/Hydra/Controller/API/checks.t.

Certainly, I've added one to the t/Hydra/Controller/Build/api.t file, since that's where the other build related api endpoints were being tested. Please see 158e856 for this addition.

I also discovered that the swagger didn't like the <<: *value expansion type anymore, so I updated the api description to just do direct dereferences in 8423bff.

@iwanders iwanders force-pushed the CORE-21733-get-info-buildsteps branch from 3f0094d to 158e856 Compare August 12, 2022 17:00
@iwanders
Copy link
Contributor Author

I do sometimes see the following warnings

( STDERR )  job 87    	(in cleanup) {UNKNOWN}: DBI Exception: DBD::Pg::st DESTROY failed: no connection to the server [for Statement "INSERT INTO users ( emailaddress, password, username) VALUES ( ?, ?, ? )"]  at /nix/store/bc7xdl0w78hmmsa7lfnpvc0rqkcbbcw8-hydra-perl-deps/lib/perl5/site_perl/5.32.1/DBIx/Class/Schema.pm line 1118.
( STDERR )  job 87    	DBIx::Class::Schema::throw_exception(Hydra::Schema=HASH(0x63c70d0), "DBI Exception: DBD::Pg::st DESTROY failed: no connection to t"...) called at /nix/store/bc7xdl0w78hmmsa7lfnpvc0rqkcbbcw8-hydra-perl-deps/lib/perl5/site_perl/5.32.1/DBIx/Class/Storage.pm line 113
( STDERR )  job 87    	DBIx::Class::Storage::throw_exception(DBIx::Class::Storage::DBI::Pg=HASH(0x64374d8), "DBI Exception: DBD::Pg::st DESTROY failed: no connection to t"...) called at /nix/store/bc7xdl0w78hmmsa7lfnpvc0rqkcbbcw8-hydra-perl-deps/lib/perl5/site_perl/5.32.1/DBIx/Class/Storage/DBI.pm line 1623
( STDERR )  job 87    	DBIx::Class::Storage::DBI::__ANON__("DBD::Pg::st DESTROY failed: no connection to the server [for "..., DBI::st=HASH(0x6958f10), undef) called at scripts/hydra-init.t line 0
( STDERR )  job 87    	eval {...} called at scripts/hydra-init.t line 0

when running the unit tests, also happens on the master branch. Don't think it's particularly concerning, but I thought I'd point it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants