atomist-skills / docker-build-skill

Atomist Skill to build and push Docker images
Apache License 2.0
1 stars 0 forks source link

Switch to datalog subscription #125

Open cdupuis opened 3 years ago

cdupuis commented 3 years ago

A subscription with the following events:

with filters (all optional):

cdupuis commented 3 years ago

@kipz @slimslenderslacks do you guys have such a subscription available that I could use?

kipz commented 3 years ago

I'm not sure there's one available as such, but they can all be crafted. I'd like to make some nice named ones or we can start low-level?

cdupuis commented 3 years ago

You can craft a low-level one. I would hide it as much as we hide graphql for known subscriptions for now. I just want to get going on this :-)

kipz commented 3 years ago

One of the key questions is: what data do you need in order to do the whole operation without any ad-hoc queries? Is that something you can enumerate? One way or another, it looks like it all hangs off a commit ref (branch or tag), but I'm wondering what else you need? So far I've been pulling back repo details token, branch and commit-sha for these sorts of things .Will that do for now?

cdupuis commented 3 years ago

Yes, I think that's all I need. Oh, and of course and api key in the subscription.

kipz commented 3 years ago

Do you mean the JWT? Why do you need that out of interest? In any case, it's in the same place.

kipz commented 3 years ago

Another question I have is about whether or not you will need the branch and/or tag name returned? It's just that with the file filter, those events can arrive after any branch or tag ref is created, and so it won't be possible to resolve to a particular tag or branch for that commit...

cdupuis commented 3 years ago

Yes, branch and/or tag name are needed.

kipz commented 3 years ago

So you're OK not getting a branch or tag when the triggering event is the fingerprint (i.e. file filter) being updated on a commit? What will you do in that instance?

kipz commented 3 years ago

We could search for and find any branch and/or tag associated with that commit?

cdupuis commented 3 years ago

Hmm, I was expecting to get called for tags and pushes. Not sure I care about the fingerprint at this point. Or should I?

kipz commented 3 years ago

That's my whole point here - when the fingerprint is added can be before, after or during the push or tag operations, so your file filter might not work at the point of push or tag. To work around this, you need to listen for those fingerprinting events too (I already do this in a skill).

kipz commented 3 years ago

Fingerprinting is done by the content-indexer skill (currently) on push.

kipz commented 3 years ago

So we can guess as to the tag or branch that you need for the commit that was just fingerprinted if you really need the branch/tag for all events.

kipz commented 3 years ago

More than likely it'll be the right one, and we can make sure it matches the branch/tag specs (if defined).

kipz commented 3 years ago
[:find
  (pull ?org [:github.org/installation-token
              :git.org/name
              :git.provider/url])

  (pull ?repo [:git.repo/source-id
               :git.repo/name
               :git.provider/url])
  (pull ?commit [:git.commit/sha])
  ?ref-name
  :in $ $before-db %
  :where
  (get-config-value "branch-filters" ["^.*$"] ?branch-filters)
  (get-config-value "tag-filters" ["^.*$"] ?tag-filters)
  ;; triggers
  (or-join [?commit ?branch-filters ?tag-filters ?ref-name]
    (tx-push-to-branch ?branch-filters ?commit ?ref-name)
    (tx-tag ?tag-filters ?commit ?ref-name)
    (and
      ;;take a guess at a branch/tag ref for this commit
      ;; but it must match the filters, if defined
      (tx-content-index ?commit)
      [?ref :git.ref/commit ?commit]
      [?ref :git.ref/name ?ref-name]
      (get-config-value "file-paths" ["Dockerfile"] ?file-paths)
      (or-join [?commit ?file-paths]
        [(empty? ?file-paths)]
        (content-index-contains? ?commit "path-exists" ?file-paths))
      (or-join [?ref ?branch-filters ?tag-filters ?ref-name]
        (and
          [?ref :git.ref/type :git.ref.type/branch]
          (array-contains? ?branch-filters ?ref-name))
        (and
          [?ref :git.ref/type :git.ref.type/tag]
          (array-contains? ?tag-filters ?ref-name)))))

  [?commit :git.commit/repo ?repo]
  [?repo :git.repo/org ?org]]
kipz commented 3 years ago

That seems to work. The file-filter stuff is a bit funky because we are guessing the ref from the commit based on the filters.

kipz commented 3 years ago

Example result:

[[{:github.org/installation-token "some-token", :git.org/name "atomisthq", :git.provider/url "https://github.com"}
  {:git.repo/source-id "225951504", :git.repo/name "web-app-cljs", :git.provider/url "https://github.com"}
  #:git.commit{:sha "8976e7077a86e2755486eb136103b26cef5c78d7"}
  "master"]]
kipz commented 3 years ago

In theory, there can be many results (hence the list of lists), but in practice, it's probably safe to assume there's just one if we have the right tx filters.

kipz commented 3 years ago

It's also worth noting that these named rules (such as tx-push-to-branch) are by no means stablised in terms of signature and naming

cdupuis commented 3 years ago

How come that ref-name doesn't have a key in the example payload?

kipz commented 3 years ago

It's positional - it's "master" in that example. Here we are pulling an individual datom, not an entity. There's nothing else useful on the ref entity TBH (it's the tag name or the branch name). We could return an entity there, but seems like more hassle than it's worth.

cdupuis commented 3 years ago

any update on this @kipz? I think I'm waiting on an updated subscription, right?

kipz commented 3 years ago

I wasn't sure if we'd closed on what to do re: refs - so basically it's likely to be the content-indexing for the file filters that triggers the skill, so you won't have branch or tag available....

cdupuis commented 3 years ago

What is going to happen when I push a tag and want to trigger only for repositories that have the specified file? In that case we need the tag name as well.

kipz commented 3 years ago

That's the same problem I'm describing. Content indexing happens after the tag/push, so when the index event comes in, we don't know which tag/branch to choose (if any). This is not a new problem - it was unsolved previously too. cc @slimslenderslacks @whostolebenfrog . It probably makes sense for us to store tags on commits (though you'll still need to choose one if there are more than one). The branch is more difficult, but I'm not convinced it's needed...WDYT?

kipz commented 3 years ago

Here's an example test-query that takes a guess at tags and branches.

If there are many tags/branches, then you'll get multiple results, one for each match.

But with the way we track branch-refs, you could get no results because the ref may well have changed with another push by the time the content is indexed.

{:query
 [:find
  (pull ?org [:github.org/installation-token
              :git.org/name
              :git.provider/url])

  (pull ?repo [:git.repo/source-id
               :git.repo/name
               :git.provider/url])
  (pull ?commit [:git.commit/sha])
  (pull ?ref [:git.ref/name])
  :in $ $before-db %
  :where
  (get-config-value "branch-filters" ["^.*$"] ?branch-filters)
  (get-config-value "tag-filters" ["^.*$"] ?tag-filters)
  ;; triggers
  (or-join [?commit ?branch-filters ?tag-filters ?ref]
    (and
      (tx-push ?commit ?ref)
      [?ref :git.ref/name ?branch-name]
      (array-contains?  ?branch-filters ?branch-name))
    (and
      (tx-tag ?commit ?ref)
      [?ref :git.ref/name ?tag-name]
      (array-contains?  ?tag-filters ?tag-name))
    (and
      ;;take a guess at a branch/tag ref for this commit
      ;; but it must match the filters, if defined
      (tx-content-index ?commit _)
      [?ref :git.ref/commit ?commit]
      (get-config-value "file-paths" ["Dockerfile"] ?file-paths)
      (or-join [?commit ?file-paths]
        [(empty? ?file-paths)]
        (content-index-contains? ?commit "path-exists" ?file-paths))
      (or-join [?ref ?branch-filters ?tag-filters]
        (and
          [?ref :git.ref/type :git.ref.type/branch]
          [?ref :git.ref/name ?branch-name]
          (array-contains? ?branch-filters ?branch-name))
        (and
          [?ref :git.ref/type :git.ref.type/tag]
          [?ref :git.ref/name ?tag-name]
          (array-contains? ?tag-filters ?tag-name)))))

  [?commit :git.commit/repo ?repo]
  [?repo :git.repo/org ?org]]

 :skill {:id "other-skill-id"
         :name "some-skill"
         :namespace "atomist"
         :team-id "AK748NQC5"
         :version "1.2.3"
         :configurations [{:name "made-up-config"
                           :parameters [{:name "branch-filters" :value ["^master*$"]}
                                        {:name "file-paths" :value ["Dockerfile"]}]}]}
 :entities [
            {:schema/entity-type :atomist.content.index/item
             :atomist.content.index/commit "$head-commit"
             :atomist.content.index/name "path-exists"
             :atomist.content.index/value "Dockerfile"}

            {:schema/entity-type :git/ref
             :schema/entity "$head-ref"
             :git.ref/type :git.ref.type/branch
             :git.provider/url "https://github.com"
             :git.ref/name "master"
             :git.ref/repo "$repo"
             :git.ref/commit "$head-commit"}

            {:schema/entity-type :git/org
             :schema/entity "$org"
             :git.provider/url "https://github.com"
             :git.org/name "atomisthq"
             :github.org/installation-token "some-token"
             :git.org/source-id "13823265"}

            {:schema/entity-type :git/repo
             :schema/entity "$repo"
             :git.provider/url "https://github.com"
             :git.repo/default-branch "master"
             :git.repo/name "web-app-cljs"
             :git.repo/source-id "225951504"
             :atomist/graphql-id "T095SFFBK_T095SFFBK_atomisthq_224272887"
             :git.repo/org "$org"}

            {:schema/entity-type :git/repo-language
             :schema/entity "$clojure"
             :git.repo-language/name "Clojure"
             :git.repo-language/repo "$repo"
             :git.provider/url "https://github.com"}

            {:schema/entity-type :git/commit
             :schema/entity "$head-commit"
             :git.provider/url "https://github.com"
             :git.commit/sha "8976e7077a86e2755486eb136103b26cef5c78d7"
             :git.commit/repo "$repo"}]}
kipz commented 3 years ago

@cdupuis so we currently this query above should be fine. In ensures that either there is at least one tag or branch HEAD ref. WDYT?

cdupuis commented 3 years ago

I'll give this a try. Thanks.

cdupuis commented 3 years ago

Trying the above subscription lets the skill fail pretty quickly. The payload is missing the required configuration (parameters and resource providers) for this to work. I disabled the subscription.

atomist[bot] commented 3 years ago

Thanks for your contribution!

This issue has been automatically marked with stale because it has not had any activity in last 50 days. It will be closed in 7 days if no further activity occurs. To prevent closing, label with defer or blocked or any of the changelog: labels.