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

Pass the artifacts checksums in the headers of the deploy request #199

Merged
merged 6 commits into from
Nov 12, 2024

Conversation

aslheyrr
Copy link
Contributor

@aslheyrr aslheyrr commented Sep 24, 2024

Description

Artifactory UI shows a warning message in the uploaded artifacts when artifact checksums aren't provided, therefore we would like to push a checksum of binaries being deployed.

Pushing checksums doesn't seem to be a well documented feature in the JFrog documentation, however here is brief reference to it https://jfrog.com/help/r/why-am-i-getting-client-did-not-publish-a-checksum-value-for-npm-packages/why-am-i-getting-client-did-not-publish-a-checksum-value...-for-npm-packages

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How has it been tested ?

Please describe the tests that you ran to verify your changes. Please also list any relevant details for your test configuration

  • Test A
  • Test B

Checklist:

  • My PR is ready for prime time! Otherwise use the "Draft PR" feature
  • All commits have a correct title
  • Readme has been updated
  • Quality tests are green (see Codacy)
  • Automated tests are green (see pipeline)

@aslheyrr
Copy link
Contributor Author

Hi @anancarv, could you please take a look at this? it seems like the token for codacy is missing

@anancarv
Copy link
Owner

anancarv commented Oct 1, 2024

Hi @anancarv, could you please take a look at this? it seems like the token for codacy is missing

Hey @aslheyrr, sure I will take a look at the MR this week. Don't worry about codacy

@aslheyrr
Copy link
Contributor Author

Hi @anancarv just a friendly reminder that this MR is awaiting review

Copy link
Owner

@anancarv anancarv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @aslheyrr, I just reviewed your PR. But I don't get why you're not using this way of deploying checksums

README.md Show resolved Hide resolved
pyartifactory/models/artifact.py Outdated Show resolved Hide resolved
@aslheyrr aslheyrr requested a review from anancarv October 22, 2024 20:19
@aslheyrr
Copy link
Contributor Author

Hi @anancarv, did you see my comments? I'd like to get your feedback so we can move forward with this feature

@aslheyrr
Copy link
Contributor Author

hey @anancarv I made some code changes in this branch, could you review them?

anancarv
anancarv previously approved these changes Nov 11, 2024
Copy link
Owner

@anancarv anancarv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm! I have only one comment and it's ready to be merged

pyartifactory/objects/artifact.py Show resolved Hide resolved
@anancarv anancarv merged commit b9cc431 into anancarv:master Nov 12, 2024
2 of 7 checks passed
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.

2 participants