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

chore: Update gitignore to exclude alembic.ini for account manager #3268

Merged
merged 3 commits into from
Dec 20, 2024

Conversation

fregataa
Copy link
Member

@fregataa fregataa commented Dec 18, 2024

ref #2689

  • Update .gitignore to exclude alembic-accountmgr.ini config file
  • Update install-dev.sh to install alembic-accountmgr.ini instead of am-alembic.ini using a more clear file name
  • Delete am-alembic.ini config file

Checklist: (if applicable)

  • Milestone metadata specifying the target backport version

@fregataa fregataa added this to the 24.09 milestone Dec 18, 2024
@fregataa fregataa self-assigned this Dec 18, 2024
@github-actions github-actions bot added the size:M 30~100 LoC label Dec 18, 2024
@fregataa fregataa changed the title Fix/update gitignore to exclude alembic fix: Update gitignore to exclude alembic Dec 18, 2024
@Yaminyam Yaminyam requested a review from Copilot December 18, 2024 09:19

Choose a reason for hiding this comment

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

Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.

Files not reviewed (1)
  • am-alembic.ini: Language not supported
@fregataa fregataa added the skip:changelog Make the action workflow to skip towncrier check label Dec 19, 2024
@achimnol achimnol changed the title fix: Update gitignore to exclude alembic chore: Update gitignore to exclude alembic Dec 20, 2024
@achimnol
Copy link
Member

I don't understand why we have to add this specific pattern to gitignore.
Should we add all patterns like these?:

/*account-manager.toml
/*manager.toml
/*agent.toml
/*storage-proxy.toml
/*webserver.conf
/*cuda-mock.toml
/*mock-accelerator.toml
/*.pants.env
/*.pants.bootstrap
/*.pants.rc
/*docker-compose.halfstack.current.yml
/*alembic.ini
/*dev.etcd.volumes.json
/*dev.etcd.installed.json
/*env-*.sh
/*dummy*.toml
/*wsproxy.toml

@achimnol achimnol force-pushed the fix/update-gitignore-to-exclude-alembic branch from 590ac38 to 431d2ab Compare December 20, 2024 13:48
@achimnol
Copy link
Member

After inspecting the code history, I found that am-alembic.ini was a part of #2688.
Let's clarify that it's part of account manager for first readers by renaming it to alembic-accountmgr.ini.

As future work, we would need to move the location of generated config files to a sub-directory to avoid conflicts of the same configuration files used in different service daemons.

@achimnol achimnol added this pull request to the merge queue Dec 20, 2024
@achimnol achimnol changed the title chore: Update gitignore to exclude alembic chore: Update gitignore to exclude alembic.ini for account manager Dec 20, 2024
Merged via the queue into main with commit 62272a9 Dec 20, 2024
19 of 21 checks passed
@achimnol achimnol deleted the fix/update-gitignore-to-exclude-alembic branch December 20, 2024 13:52
Copy link

Backport to is failed. Please backport manually.

achimnol added a commit that referenced this pull request Dec 20, 2024
lablup-octodog pushed a commit that referenced this pull request Dec 27, 2024
Co-authored-by: Joongi Kim <[email protected]>
Backported-from: main (24.12)
Backported-to: 24.09
Backport-of: 3268
github-merge-queue bot pushed a commit that referenced this pull request Dec 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:M 30~100 LoC skip:changelog Make the action workflow to skip towncrier check
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants