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

Check if the internal project and repos exist before creating them #1010

Merged
merged 5 commits into from
Aug 16, 2024

Conversation

minwoox
Copy link
Member

@minwoox minwoox commented Aug 13, 2024

Motivation:
Central Dogma server currently attempts to create internal projects and repositories during starting up, even if they already exist. This is unnecessary for most scenarios, except when setting up a new cluster.

Modifications:

  • Added checks to verify if the internal project and repositories exist before attempting to create them.
  • Propagated the read-only exception to the caller if it's raised while creating the internal project.
    • It doesn't make sense to continue to run the cluster because the internal project must exist when the cluster gets back to non read-only mode.

Result:

  • Central Dogma servers now checks for the existence of internal projects and repos before attempting to create them.

Motivation:
Central Dogma server currently attempts to create internal projects and repositories during starting up, even if they already exist.
This is unnecessary for most scenarios, except when setting up a new cluster.

Modifications:
- Added checks to verify if the internal project and repositories exist before attempting to create them.
- Propagated the read-only exception to the caller if it's raised while creating the internal project.
  - It doesn't make sense to continue to run the cluster because the internal project must exist when the cluster gets back to non read-only mode.

Result:
- Central Dogma servers now checks for the existence of internal projects and repos before attempting to create them.
"mirroringEnabled": true,
"numMirroringThreads": null,
"maxNumFilesPerMirror": null,
"maxNumBytesPerMirror": null,
Copy link
Member Author

Choose a reason for hiding this comment

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

Let me remove these broken properties for now.
We will introduce a build process to prevent this sort of mistakes later as described in
#1003

@minwoox minwoox added this to the 0.68.1 milestone Aug 14, 2024
throw new Error("failed to initialize an internal project: " + projectName, peeled);
if (!projectManager.exists(projectName)) {
try {
executor.execute(createProject(creationTimeMillis, Author.SYSTEM, projectName))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: Could you wrap createProject() and createRepository() with forcePush() command to make it work in REPLICATION_ONLY mode?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good suggestion. Applied. 👍

@ikhoon
Copy link
Contributor

ikhoon commented Aug 16, 2024

The test failure in CI will be fixed by #1016

Copy link
Contributor

@ikhoon ikhoon left a comment

Choose a reason for hiding this comment

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

Thanks, @minwoox! 👍👍

@ikhoon ikhoon merged commit a4e5893 into line:main Aug 16, 2024
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants