I don't have much to add, just that I agree with Kamil. I see some minor
problems even in what he wrote, but that's well beyond what I think we
should be solving now.


On Tue, Jan 10, 2017 at 5:19 PM, Kamil Paral <kpa...@redhat.com> wrote:

> > Couldn't we use the task ID as the primary identifier but use the srpm
> > for readability sake since there is no "build" NVR for scratch builds?
> Systems like new hotness will need to query according to the task ID,
> though (to get reliable results). So we're talking about hacking just
> resultsdb *frontend* here, e.g. by having "item: task_id" and
> "item_display: nvr" in the results yaml. I don't like it much. Searching in
> the frontend would have to search both in item and item_display.
> Or we could use our existing "note" field to put NEVR inside, and it would
> be easily visible in the frontend without any ugly hacks. People would have
> to know that if they want to search by NEVR for scratch builds, they need
> to put the NEVR in the note search field (we'd have to add it).
> I assume you proposed using task id as primary identifier just to scratch
> builds (but not standard builds). If you also mean it for standard builds,
> then scheduling tasks locally starts to be quite inconvenient (for each
> build you want to run your task on, you need to find its task id first;
> it's hard to re-run something from shell history). We would also be
> changing something that's a de-facto standard in our infra (using NEVR as a
> unique identifier across projects).
> > Either that or make the primary ID a tuple of (srpm name, koji task
> > start time) either stored that way or rendered as a string e.g
> > foo-1.2-3.fc99.src.rpm built at 201701092100 (YYYYMMDDHHMM) would become
> > foo-1.2-3.fc99-201701092100.
> The same problem with inconvenient local execution. The command line
> starts to be hard to read (in tutorials, etc).
> Also, the srpm name doesn't have to be anything reasonable, Koji will
> happily accept anything (it doesn't care, for scratch builds). So we can
> easily receive "1.srpm" or "my-new-patch.srpm" from the fedmsg (we actually
> tried this [1]). Deriving any useful info from this input is probably a
> mistake. We would have to download the srpm and look into the included spec
> file to reliably decide which package this is related to.
> [1] https://apps.fedoraproject.org/datagrepper/id?id=2017-
> 2fe001d2-32d4-434a-b3a9-29ab31eebbb0&is_raw=true&size=extra-large
> >
> > > During a discussion with Kamil, a few solutions were mentioned (none
> > > of them is pretty):
> > >
> > > 1. We can ask koji developers if there is a way to add a method that
> > > would return all koji scratch builds for a given NVR - we would then
> > > take the latest one and work with it.
> >
> > How would we get the NVR if there's no build identifier on scratch
> > builds? Are you talking about the SRPM name in the fedmsg?
> Yes. Not from the fedmsg, but from the Koji task (the fedmsg gets this
> value from the srpm filename included in the Koji task, I believe). But see
> the problem described above with srpm file naming (can be arbitrary). So
> even better would be if Koji could search and return scratch build info
> containing the metadata of the srpm or from the spec file included.
> But I'm honestly not sure it is a good idea even if they wanted to
> implement it. More on that below.
> >
> > > 2. We can use "koji download-task" which works for both. That would
> > > mean koji_build item type would eat koji task IDs instead of NVRs.
> > > This would lead to having koji task IDs in resultsdb instead of NVR's
> > > which kills readability. Unless libtaskotron finds the NVR from the
> > > koji task ID in the case of "real" build" and stores it in a field in
> > > resultsdb...Also, we need NVRs for fedmsgs. So add code to the fedmsg
> > > layer that would take care of somehow adding a NVR to fedmsg of
> > > completed scratch builds tasks...
> >
> > Can't we tell the difference between an NVR and a task ID just by the
> > form that the content takes? Wouldn't the following work:
> >
> >   1. attempt to parse input as NVR, if there are no issues, assume that
> >      it's a build ID
> >   2. if the NVR parse fails, check to see if the input is an int. if
> >      assume that it's a koji task ID
> The reverse might be easier, but yes, that's of course possible. There are
> other caveats, though.
> >
> > It'd mean that our koji downloading code would get more complicated but
> > it seems like nothing that more unit tests couldn't mitigate. Unless
> > I'm mis-thinking here, the changes needed to pull this off in trigger
> > would be even smaller.
> I'm afraid our code could get infested with if clauses pretty soon. This
> is not just about downloading rpms in the koji directive. We have more koji
> build-related functionality and we'll sure get more in the future. For
> example, we currently support "download_latest_stable", but that might be
> very tricky for scratch builds with "1.srpm" files (which package is
> that?). If we ever needed to work with package epoch, it is unavailable for
> scratch builds (since there is no build info object to be returned).
> Another example, we have dist-git directive. Without clear identification
> of the package you can't use dist-git directive. Even if we download 1.srpm
> and look into it, you can still find something that's not available in
> dist-git (because you can build something in Koji that is not packaged in
> Fedora).
> What I'm trying to illustrate here is that this is not just a few if
> clauses in a single piece of code (koji utils). This can influence the
> whole workflow through the formula. Some directives might return different
> data based on whether it is a regular build or a scratch build. Some might
> completely fail to work (distgit directive). And you might want to have
> different standards and approaches for scratch builds.
> As an example, you might want to use distgit directive to download
> rpmlint.config for that particular package when running rpmlint. If that
> git repo doesn't exist, you want to error out (should never happen). But
> for scratch builds, it's helpful if you figure out which package it is and
> the config file is used, but you don't mind if such a repo doesn't exist -
> the rpmlint output without any personalized rpmlint.config is still
> valuable. How do you implement that? Will we depend on heuristics
> (recognizing standard vs scratch build) and hard-code whether it errors out
> or not? We can, but will we be always to do the right thing? What if some
> tasks have different preferences? Will we not end up with monster code full
> of if-then-else? It seems to me that it's a better approach to allow the
> task to decide what it wants to do and fully inform it in advance whether
> this is a standard build or a scratch build. That means using different
> item_type and therefore different formula. The advantage is that you can
> easily implement something like "fail_on_error: True/False" option for a
> directive and the task can set what it needs. Or the task will know whether
> it's possible to retrieve epoch for a build and will act accordingly.
> As a last example, different logic might be desirable for certain tasks
> even in the "task code" (not libtaskotron code) itself. With abicheck, we
> currently compare new koji build with the latest build in stable updates.
> But for scratch builds, different approach might be desired - e.g.
> comparing to the last standard/scratch build, regardless of the koji tag it
> is in. We actually talked about very this topic the last time I saw Sinny
> and Dodji. If we decided to merge standard builds and scratch builds into a
> single workflow, we don't allow tasks to take a different approach even if
> they wanted (unless they go into such extents like determining whether the
> current "item" is a standard or a scratch build, and then emulating e.g. a
> koji directive in their own code). Btw, what about tasks that want/need to
> run exclusively on just standard or just scratch builds?
> >
> > > 3. We can add a new action to the koji directive "download_task". And
> > > then have for each koji task two formulas, one for scratch build
> > > (using "download_task") and one for "real" build" (using
> > > "download_build). This would require writing code for supporting
> > > multiple formulas for one task and would result in having two almost
> > > identical formulas...
> >
> > I'm not a huge fan of 3 if there is any other reasonable way to do
> > things. Code duplication is not the way to go.
> I've been thinking about this and I actually like it the most. I already
> described much of the issues with compounding scratch+standard builds in
> the text above, but let me expand on that. I currently believe having a
> separate koji_build and koji_scratch_build type is the best and the most
> technically clean way to go. These two events look very similar on the
> first (and second and third) glance, but they are implemented very
> differently. There's a good reason that their fedmsgs look not alike at
> all. Scratch builds have no restrictions on NEVRs, no uniqueness
> constraints, no dist-git constraints (anything can be built), they do not
> have any API (except a completely generic "task" API), we can't get any
> structured metadata about them. They are very different beasts. Compare
> this to e.g. DNF repo compose, ostree compose, and docker compose. They are
> all "composes", but sufficiently different to get different item types and
> different handling in directives (or probably even some exclusive
> directives). I believe this is the same case.
> The code duplication doesn't have to be a problem, I think. In package
> distgit, you'll have koji_build/mytest/runtask.yaml and
> koji_scratch_build/mytest/runtask.yaml. You can symlink any code between
> dirs, if your task is able to handle both. Or you can have separate code,
> if your task needs it (!). The formulas are duplicated, yes, but not
> really. First, they don't change much, so it's not really a problem, and
> second you might want to have a different behavior anyway (e.g. remove
> dist-git directive to download rpmlint.conf, since it's problematic for
> scratch builds).
> For non-distgit (generic) tasks, there would be two formulas under
> different names in the same repo (say runtask.yaml and
> runtask_scratch.yaml). We would configure trigger to run the task with
> 'runtask.yaml' for koji builds and 'runtask_scratch.yaml' for scratch
> builds (that should be hopefully trivial). And this is in line with our
> future ideas how scheduling should work - users should be able to configure
> when to run their task in a web UI, and a formula name would be part of
> that. That makes it flexible and would allow them to do awesome things,
> like experiment with a new formula while keeping the old formula still
> working. In short, I believe we want to go this route (multiple formula
> support) anyway. Having it currently "hard-coded" in trigger is not
> necessarily a bad thing, it's a step forward towards having it in a
> database.
> Ugh, such a long email. Bad kparal.
> _______________________________________________
> qa-devel mailing list -- qa-devel@lists.fedoraproject.org
> To unsubscribe send an email to qa-devel-le...@lists.fedoraproject.org
qa-devel mailing list -- qa-devel@lists.fedoraproject.org
To unsubscribe send an email to qa-devel-le...@lists.fedoraproject.org

Reply via email to