Change provenance to filter relevant LIDVIDs on "archive_status in [archived, certified]" #312
Conversation
also renames some variables
…ther than excluding status==staged this aligns provenance script's behaviour with the API's default behaviour and prevents problems where archive_status of a product is erroneously nulled in db fixes #305
alexdunnjpl
requested review from
jimmie,
nutjob4life,
jordanpadams and
tloubrieu-jpl
![sonatype-lift[bot]](https://avatars.githubusercontent.com/in/9843?s=60&v=4){kind=small}
|
@tloubrieu-jpl @jimmie I know we talked about having to update deployment stuff to support this change, but the command is defined in the driver so we should be good to seamlessly update, yeah? |
|
since provenance is executed as a ECS/Fargate stand-alone, we need to create a new docker image that contains your updated script, push it up to ECR and update the Fargate task to point to the new image |
|
We could make the docker image build part of the CICD although it does not push to ECR but on docker hub. Let's do that manually for now. I will create a separate ticket for the CICD to be able to push on ECR. |
|
If you're suggesting the script be in its own repository wrt docker for the sake of not having the entire repo in the image, the docker file clones the repo but extracts the script and driver, discarding the rest. |
|
I did not have technIcal concerns but more an organization concern. I would like us to have one repository per service/component and not generate multiple docker images from a single repository. In addition the languages used in registry-api and provenance script are different. I would not keep it in the same repo. It is more a rule of thumb than anything though. |
IGNORE THIS PR UNTIL #311 IS MERGED
There's a minor conflict it seemed simpler not to bother dealing with. This PR is actually only ce7f755, which is a trivial diff.
🗒️ Summary
Previously, filtering occurred on "archive_status != staged", which was causing API issues where archive_status was erroneously nulled.
⚙️ Test Data and/or Report
Manually tested only
♻️ Related Issues
Fixes #305