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

Fingerprint, Updater script for flyte console. #471

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

mr-mosi
Copy link
Contributor

@mr-mosi mr-mosi commented Apr 26, 2024

Hi dear tsunami team,

Regarding to issue #426

Please note that, Flyte project has various products but all of them use flyteconsole as their dashboard. So as we need web fingerprints of this projects I run flyteconsole to get fingerprints. I used released versions because they were the most downloaded. and there is one or two versions which the release version not identical with git version, so they were ignored.

bests.

@mr-mosi mr-mosi changed the title updater script, asset files and also fingerprint of flyte console added. Fingerprint, Updater script for flyte console. Apr 27, 2024
@lokiuox
Copy link

lokiuox commented May 24, 2024

Hi @mr-mosi, thank you for your contribution!

I'm reviewing your plugin, but I noticed that the versions.txt file you supplied only contains 5 versions, up to v1.1.3, which is a release from June 2022. From the GitHub repo and the container registry you're using in your script I can see there are way more releases, and the last tagged docker image is v1.12.0. Is there a reason why all these versions are missing?

As a general rule, the plugin should be able to fingerprint as many versions as possible, especially the latest versions.

@mr-mosi
Copy link
Contributor Author

mr-mosi commented May 24, 2024

Hi @lokiuox thank you for your review!

That's good catch! the version.txt file is wrong and I fix it after this message. I just did test on some versions before and I sent a testing file by mistake. so I must fix fingerprints as well. I will do it soon.

I get versions from this address. and you can check them.

@mr-mosi
Copy link
Contributor Author

mr-mosi commented May 24, 2024

Hi @lokiuox ,
I fixed version.txt file and updated the fingerprints again.
two version v1.10.7 and v1.10.6 are removed because they don't have identical git tag to checkout.

thanks!

Copy link

@lokiuox lokiuox left a comment

Choose a reason for hiding this comment

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

LGTM - Approved
@maoning you can merge it.

Reviewer: Savio, Doyensec

Plugin: Flyte Console Fingerprints
Feedback: Besides the version.txt file being initially incomplete, the script worked first try and the quality is good.
Drawbacks: None

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.

None yet

2 participants