#58 Provide artifact.id field unique among all artifacts of the same type
Merged 4 years ago by mvadkert. Opened 4 years ago by happz.
fedora-ci/ happz/messages pull-request-id-not-unique  into  master

file modified
+3 -2
@@ -11,13 +11,13 @@ 

          enum:

              - fedora-dist-git

          type: string

-     id:

+     pr_id:

          description:

              Human readable pull request ID.

          examples:

              - 123

          type: integer

-     uid:

+     id:

          description:

              Pull request id unique across the whole Pagure instance.

          examples:
@@ -52,6 +52,7 @@ 

  required:

      - type

      - id

+     - pr_id

      - comment_id

      - commit_hash

      - repository

To remain consistent with the rest of the artifacts:id tells one artifact of the same type from another, therefore must be unique. It cannot be unique within a repository/project, because that leads to a huge pile of the same type and the same id, 1 (since many projects one day get their first pull request). It makes no sense to not use unique value for id property, that would require all consumers to add extra exception when decoding artifact type and id: for all artifacts, type+id pair identifies the particular artifact, just pull requests are the exception and one has to examine uid field...

Therefore renamed uid to id to provide the unique ID value, and id renamed to pr_id to carry the project-specific ID of the pull request.

yeah, makes sense, I wonder if we need to change our recent changes to our pagure builder @mkluson ?

Metadata Update from @mvadkert:
- Pull-request tagged with: review-needed

4 years ago

@mvadkert when this PR is merge, we have to do the same renaming (uid => id, id => pr_id) in messages we emit

And we will need to also fix pagure's UMB relay ....

So, I am setting as blocked on PRs on the above ^

Metadata Update from @mvadkert:
- Pull-request tagged with: blocked

4 years ago

@mvadkert what is this blocked on? Can you link to the ticket?

Ping @mvadkert, should we close this one without merging? It looks like it might be stuck.

Nope, I would add this, for the whole infra we still use 0.1.x spec anyway. Once we migrate to 0.2.x, we should migrate to the latest spec .. so we can directly incorporate this change

Metadata Update from @mvadkert:
- Pull-request untagged with: blocked, review-needed
- Pull-request tagged with: review-done

4 years ago

Pull-Request has been merged by mvadkert

4 years ago
Metadata