Skip to content

Conversation

@chenmingyong0423
Copy link
Contributor

@chenmingyong0423 chenmingyong0423 commented May 26, 2025

Renamed data/seed_2025_05_16.json to data/seed.json and updated Dockerfile

Motivation and Context

When cloning the repository and building the project using:

go build ./cmd/registry
.\\registry.exe

the application fails to start because the default seed file path (data/seed.json) does not exist in the repo:

Failed to read seed file: failed to read file: open data/seed.json: The system cannot find the file specified.

How Has This Been Tested?

go build ./cmd/registry
.\\registry.exe // ./registry

Breaking Changes

None

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@yshngg
Copy link
Contributor

yshngg commented May 26, 2025

Hi @chenmingyong0423 , why not simply rename the seed_2025_05_16.json file to seed.json?

@chenmingyong0423
Copy link
Contributor Author

@yshngg The current seed file is named seed_2025_05_16.json, and I wasn’t sure if there’s a specific versioning or naming convention behind it. To avoid unintentionally breaking that design decision, I chose to update the config and Docker setup to match the existing file rather than renaming it to seed.json.

@yshngg
Copy link
Contributor

yshngg commented May 26, 2025

@chenmingyong0423 I understand your point, but I think this PR might reduce flexibility. The current seed file is named seed_2025_05_16.json. If updates are made later and a new file like seed_2025_05_26.json is created, we would need to update the default path to import seed file again.

If we don't create new seed files, what's the difference between filenames seed_2025_05_16.json and seed.json?

@chenmingyong0423
Copy link
Contributor Author

@yshngg I actually agree that renaming seed_2025_05_16.json to seed.json would be a cleaner and simpler solution. However, I'm not sure if there was any specific reason or versioning intent behind the original filename. If not, standardizing it as seed.json makes perfect sense. Looking forward to hearing the maintainers’ thoughts.

@chenmingyong0423
Copy link
Contributor Author

chenmingyong0423 commented May 26, 2025

@yshngg I'd like to check — are you the main reviewer or maintainer of this project? If so, I’m happy to adjust the PR based on your suggestions.

@chenmingyong0423
Copy link
Contributor Author

@yshngg It looks like you're the main reviewer or maintainer — I’ve updated the changes according to your suggestions. Thanks for the input!

@chenmingyong0423 chenmingyong0423 changed the title fix: Update default SeedFilePath to existing seed file Fix missing seed.json on startup by renaming file and updating Dockerfile May 26, 2025
@yshngg
Copy link
Contributor

yshngg commented May 26, 2025

@yshngg I'd like to check — are you the main reviewer or maintainer of this project? If so, I’m happy to adjust the PR based on your suggestions.

Oops, sorry for the confusion — I'm just interested in the project and not a reviewer or maintainer.

@chenmingyong0423
Copy link
Contributor Author

@yshngg I'd like to check — are you the main reviewer or maintainer of this project? If so, I’m happy to adjust the PR based on your suggestions.

Oops, sorry for the confusion — I'm just interested in the project and not a reviewer or maintainer.

Alright!

@chenmingyong0423 chenmingyong0423 changed the title Fix missing seed.json on startup by renaming file and updating Dockerfile fix: fix missing seed.json on startup by renaming file May 27, 2025
@mukteshkrmishra
Copy link

This is actually nice.

domdomegg
domdomegg previously approved these changes Aug 6, 2025
Copy link
Member

@domdomegg domdomegg left a comment

Choose a reason for hiding this comment

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

LGTM, thanks 🙌

@domdomegg
Copy link
Member

// rebasing on top of main...

@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@domdomegg domdomegg merged commit 26b85d7 into modelcontextprotocol:main Aug 6, 2025
7 checks passed
domdomegg added a commit that referenced this pull request Aug 7, 2025
Renamed `data/seed_2025_05_16.json` to `data/seed.json` and updated `Dockerfile`

## Motivation and Context

When cloning the repository and building the project using:

```bash
go build ./cmd/registry
.\\registry.exe
```

the application fails to start because the default seed file path
(`data/seed.json`) does not exist in the repo:

```
Failed to read seed file: failed to read file: open data/seed.json: The system cannot find the file specified.
```

## How Has This Been Tested?

```bash
go build ./cmd/registry
.\\registry.exe // ./registry
```

## Breaking Changes

None

## Types of changes
<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update

## Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [x] I have read the [MCP
Documentation](https://modelcontextprotocol.io)
- [x] My code follows the repository's style guidelines
- [x] New and existing tests pass locally
- [ ] I have added appropriate error handling
- [ ] I have added or updated documentation as needed

Co-authored-by: Adam Jones <[email protected]>
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.

5 participants