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

Allow specifying jobset inputs for flake builds for Hydra plugins #922

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Ma27
Copy link
Member

@Ma27 Ma27 commented Apr 24, 2021

It seems to be a common pattern to configure Hydra plugins on a
per-jobset basis with jobset inputs. Since these are not needed anymore
for flakes, it's also not possible anymore to use plugins for flake
jobsets.

This patch changes this and adds a warning to avoid confusion. In the
future, we may want to restrict jobset inputs for flakes to string
values only.

cc @grahamc

@Ma27
Copy link
Member Author

Ma27 commented Apr 26, 2021

also CCing @edolstra whoi originally implemented flake support for Hydra.

@Ma27 Ma27 force-pushed the jobset-inputs-for-flakes branch from 4292d49 to 18ff0c7 Compare May 5, 2021 08:45
@grahamc
Copy link
Member

grahamc commented May 5, 2021

I'm not sure it is a good idea to bring back jobset inputs like this. I wonder if it would make more sense to have plugin configuration be a different thing? What plugins use jobset inputs for config?

@grahamc
Copy link
Member

grahamc commented May 5, 2021

Concretely, I can imagine someone being confused about why their jobset inputs aren't available to their jobset. Also, it causes perhaps internal confusion about if the inputs should be fetched, or input values tracked over time... overall I'm not keen on this. I wonder if we could create a different way to provide the needed configuration to plugins, or if the plugins should adapt in a different way.

@Ma27
Copy link
Member Author

Ma27 commented May 5, 2021

I wonder if it would make more sense to have plugin configuration be a different thing?

@grahamc plugins for gitea/gitlab/bitbucket also require a git input to obtain e.g. the git revision. So in the end we'd need some input fetching either way AFAIU.

I wonder if we could create a different way to provide the needed configuration to plugins, or if the plugins should adapt in a different way.

I could think of a "plugin configuration", but I'm uncertain about the following aspects:

  • should this be a purely UI-related change (with good documentation in the code, of course to avoid confusion for contributors)
  • how should "Legacy" jobsets (not an expression of opinion, just using the terminology of Hydra btw :D) behave then? Should these mix up the plugin configuration with jobset inputs (that would speak for not separating the datastructure IMHO) or should we segregate these kinds of inputs there as well? (That would be a pretty big breaking change and something I would not want to backport to a stable NixOS btw).

It seems to be a common pattern to configure Hydra plugins on a
per-jobset basis with jobset inputs. Since these are not needed anymore
for flakes, it's also not possible anymore to use plugins for flake
jobsets.

This patch changes this and adds a warning to avoid confusion. In the
future, we may want to restrict jobset inputs for flakes to string
values only.
@Ma27 Ma27 force-pushed the jobset-inputs-for-flakes branch from 18ff0c7 to 36e7c9e Compare May 8, 2021 14:54
@Ma27
Copy link
Member Author

Ma27 commented May 8, 2021

@grahamc rebased onto latest master.

@Ma27
Copy link
Member Author

Ma27 commented Aug 24, 2021

@grahamc any updates here? :)

@Ma27
Copy link
Member Author

Ma27 commented Nov 5, 2021

@grahamc given your latest comments in the Hydra matrix channel, do you consider this mergeable now? :)

Ma27 added a commit to Ma27/terraform-provider-hydra that referenced this pull request Nov 14, 2021
Needed for Hydra instances using NixOS/hydra#922
Ma27 added a commit to Ma27/terraform-provider-hydra that referenced this pull request Nov 14, 2021
Needed for Hydra instances using NixOS/hydra#922
@Ma27
Copy link
Member Author

Ma27 commented Jan 2, 2022

@grahamc @cole-h anything I can currently do to get this one merged? :)

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.

None yet

2 participants