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

Add a tmp directory #75

Merged
merged 2 commits into from
Sep 3, 2024
Merged

Conversation

GuillaumeBourque-QC
Copy link
Contributor

In order to run this in k8s or OpenShift, it's best to create a directory to write the files, other than the current directory where the python code is located.

This is my proposal, all comments are welcome!

Signed-off-by: GuillaumeBourque-QC <[email protected]>
@SylvainMartel
Copy link

@WadeBarnes or @swcurran , could you review this small PR?
Guillaume is also working on issue #62

@WadeBarnes
Copy link
Member

While I like the idea of having a separate folder for the data I'm not really a fan of using a generic name like tmp. In this case the folder is specific to genesis files so I would recommend some thing like genesis or if we want to stick with something more generic data.

@GuillaumeBourque-QC
Copy link
Contributor Author

Thanks for the fast response,

Well I'm almost 100 % with you.. But let me bring more info, these genesis file are always download and are be defaut temporary if I understand correctly ?

In OpenShift I did a temporary mount point (emptyDir: {} ) specific for this genesis directory that will be lost when the container is restart. The log directory is persistant, so this one will endup on on real volume but for the genesis file I thought that tmp would make sense ?

data is usually more permanent, and I'm not sure we need to keep those file form a k8s#openshift perspective.

So please let me know waht would best make sens for the genesis files

Best, Guillaume

@GuillaumeBourque-QC
Copy link
Contributor Author

Hi @WadeBarnes if data makes more sense I'll push this in a few minutes.

@WadeBarnes
Copy link
Member

Your raise some good points, yes, they can be considered temporary, but you could also decide to persist them longer term. Perhaps a better name for the directory would be cache.

@GuillaumeBourque-QC
Copy link
Contributor Author

I rename tmp by cache and pushed it @WadeBarnes anything else ? If all is fine please let me know if we can push a new docker images with those changes so that I can test in our openshift env.

@WadeBarnes WadeBarnes merged commit ec32ff5 into hyperledger:main Sep 3, 2024
1 check passed
@GuillaumeBourque-QC GuillaumeBourque-QC deleted the add-tmp-dir branch September 3, 2024 21:28
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.

3 participants