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

Moving all scripts to git #957

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

shahdyousefak
Copy link
Contributor

@shahdyousefak shahdyousefak commented Feb 11, 2025

Please ensure you've followed the checklist and provide all the required information before requesting a review.
If you do not have everything applicable to your PR, it will not be reviewed!
If you don't know what something is or if it applies to you, ask!

Don't delete below this line.


Required Information

  • I referenced the issue addressed in this PR. --> in an effort to consolidate all material to Github, we've migrated these scripts here for anyone who might find these handiful. I generalized them as much as possible, and included any API keys or hardcoded paths in .gitignore
  • I described the changes made and how these address the issue.
  • I described how I tested these changes. --> I ran all these scripts with bash from our project.

Coding/Commit Requirements

  • I followed applicable coding standards where appropriate (e.g., PEP8)
  • I have not committed any models or other large files.

New Component Checklist (mandatory for new microservices)

  • I added an entry to docker-compose.yml and build.yml.
  • I created A CI workflow under .github/workflows.
  • I have created a README.md file that describes what the component does and what it depends on (other microservices, ML models, etc.).

OR

  • I have not added a new component in this PR.

@shahdyousefak shahdyousefak linked an issue Feb 11, 2025 that may be closed by this pull request
4 tasks
@shahdyousefak shahdyousefak requested review from jeffbl and removed request for jaydeepsingh25 February 11, 2025 16:16
@jeffbl jeffbl requested review from JRegimbal and jeffbl and removed request for jeffbl February 14, 2025 17:31
@jeffbl jeffbl assigned JRegimbal and unassigned jeffbl Feb 14, 2025
@jeffbl
Copy link
Member

jeffbl commented Feb 14, 2025

I added a README.md and added/modified the documentation in each of the scripts

@shahdyousefak I didn't review your abstraction into env files, but I assume we'll quickly see if all that works once deployed, and these are not showstoppers since they are only support scripts, so I'm not too worried.

@shahdyousefak Is the take that we get this merged, then update unicorn and pegasus manually to switch over to these? (update cron path, links in /var/docker/image etc? If so, we should be careful to do the merge and the update in close order and not leave that hanging since a bunch of stuff will break.

@JRegimbal assigining to you for a brief look since you created many of these scripts. The bar for these is lower than for code meant for production.

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.

Move /var/docker/image/bin scripts to git
4 participants