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

Trend pytmv1 0.6.1 #20932

Closed
wants to merge 9 commits into from
Closed

Trend pytmv1 0.6.1 #20932

wants to merge 9 commits into from

Conversation

shaqnawe
Copy link
Contributor

@shaqnawe shaqnawe commented Oct 13, 2023

Status

In Progress.

Related Content Pull Request

demisto/content#30157

Related Issues

Related: link to the issue

Description

Created new demisto-docker image with custom pytmv1 package.

@github-actions
Copy link
Contributor

Docker Image Ready - Dev

Docker automatic build at CircleCI has completed. The docker image is available as an artifact of the build.

Follow the output from the build on how to load the image locally for testing:

@JudahSchwartz
Copy link
Collaborator

do we need to lock on this specifc version? If yes we should lock in the dependabot config also by telling dependabot to ignore it.
You can find other examples in .github/dependabot.yml

@JudahSchwartz
Copy link
Collaborator

JudahSchwartz commented Oct 15, 2023

would reccomend locking on * and using dynamic versioning to express the version in the tag instead of putting the version in the name of the image

@RotemAmit RotemAmit requested review from ilappe and removed request for RotemAmit October 16, 2023 07:13
@RotemAmit RotemAmit assigned ilappe and unassigned RotemAmit Oct 16, 2023
@ilappe
Copy link
Contributor

ilappe commented Oct 16, 2023

@JudahSchwartz
i recommend to add the module pytmv1 to the our new image vendors-sdk
instead of new image, as this is small module
WDYT?

@ilappe
Copy link
Contributor

ilappe commented Oct 17, 2023

Hi @shaqnawe
i will add the module pytmv1 to the dependencies list in the image vendors-sdk
in docker/vendors-sdk/pyproject.toml
so i close this PR

@ilappe ilappe closed this Oct 17, 2023
@ilappe ilappe reopened this Oct 17, 2023
@ilappe
Copy link
Contributor

ilappe commented Oct 17, 2023

due to dependencies collision the pytmv1 failed to be added to the vendors-sdk
@JudahSchwartz do we have something to do to fix it or we will need the new docker image for this module?
image

@kgal-pan kgal-pan self-assigned this Oct 18, 2023
@shaqnawe
Copy link
Contributor Author

shaqnawe commented Oct 19, 2023

Hi @shaqnawe i will add the module pytmv1 to the dependencies list in the image vendors-sdk in docker/vendors-sdk/pyproject.toml so i close this PR

@ilappe Is there something I need to do on this?

@kgal-pan
Copy link
Contributor

kgal-pan commented Oct 22, 2023

Looks like we have some hard-coded requests versions used by pytmv1 and pycti. Since pycti is locked because of #19991, I think the fastest way forward is to create a new image specifically for pytmv1 in this PR.

@shaqnawe - Can you please add a verify.py script to fix https://github.com/demisto/dockerfiles/actions/runs/6502596031/job/17714329333?pr=20932? See https://github.com/demisto/dockerfiles?tab=readme-ov-file#adding-a-verifypy-script for instructions.

@kgal-pan kgal-pan added the native image approved related changes to the native docker were needed and occurred. label Oct 22, 2023
@ilappe
Copy link
Contributor

ilappe commented Oct 22, 2023

Hi @shaqnawe i will add the module pytmv1 to the dependencies list in the image vendors-sdk in docker/vendors-sdk/pyproject.toml so i close this PR

@ilappe Is there something I need to do on this?

Hi @shaqnawe
as mentioned above - we have issue to add pytmv1 to an existing image so we will need to create a separate image for that
please follow the comments above

@JudahSchwartz JudahSchwartz added native image approved related changes to the native docker were needed and occurred. and removed native image approved related changes to the native docker were needed and occurred. labels Oct 22, 2023
@shaqnawe
Copy link
Contributor Author

@ilappe Please let me know if this is correct, thank you.

@kgal-pan
Copy link
Contributor

kgal-pan commented Oct 24, 2023

@shaqnawe - In continuation of #20932 (comment), can you please rename the folder to docker/pytmv1 (remove -061 from the name):

mv docker/pytmv1-061 docker/pytmv1

@ilappe
Copy link
Contributor

ilappe commented Oct 26, 2023

@shaqnawe - thanks!
can you release the lock of pytmv1 = "==0.6.1" in the Pipfile as mentioned in this comment?

Copy link
Contributor

@ilappe ilappe left a comment

Choose a reason for hiding this comment

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

Great!

@shaqnawe
Copy link
Contributor Author

shaqnawe commented Oct 27, 2023

@ilappe There is a new update for pytmv1. Do I need to update the image and push or will that be automatically done?

@ilappe
Copy link
Contributor

ilappe commented Oct 29, 2023

@ilappe There is a new update for pytmv1. Do I need to update the image and push or will that be automatically done?

@shaqnawe - we will update it
@kgal-pan
can you do re-lock to take the newer version mentioned?

@kgal-pan
Copy link
Contributor

@kgal-pan can you do re-lock to take the newer version mentioned?

47e1f4d

@ilappe
Copy link
Contributor

ilappe commented Oct 29, 2023

will be merged via #21608

@ilappe ilappe closed this Oct 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Contribution native image approved related changes to the native docker were needed and occurred.
Projects
None yet
5 participants