Skip to content

Conversation

@jrconlin
Copy link
Member

@jrconlin jrconlin commented Aug 19, 2025

This collects various improvements and pointers from someone getting started with Merino.

References

JIRA: DISCO-3666

Description

This collects various improvements and pointers from someone getting started with Merino.

PR Review Checklist

Put an x in the boxes that apply

  • This PR conforms to the Contribution Guidelines
  • The PR title starts with the JIRA issue reference, format example [DISCO-####], and has the same title (if applicable)
  • [load test: (abort|skip|warn)] keywords are applied to the last commit message (if applicable)
  • Documentation has been updated (if applicable)
  • Functional and performance test coverage has been expanded and maintained (if applicable)

┆Issue is synchronized with this Jira Task

This collects various improvements and pointers from someone getting
started with Merino.
Copy link
Contributor

@tiftran tiftran left a comment

Choose a reason for hiding this comment

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

thanks JR!

@jrconlin
Copy link
Member Author

@tiftran Heh, thanks, but not quite done yet. (Ideally, there should be a test framework in here as well.)

I'll convert from draft once it's ready to go and add reviewers.

Thanks for the drive-by!

Better understanding of the Data models, manifests, and code structures.
Copy link
Contributor

@Herraj Herraj left a comment

Choose a reason for hiding this comment

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

I noticed a few TODOs but otherwise looking good! Easy to understand and follow and concise wording!

- `development.toml`, etc. - The platform specific configurations to use. These will eventually be replaced by a single, composed `platform.toml`(name TBD).

Validators for the configuration options are stored in the `./merino/configs/__init__.py` file

Copy link
Contributor

Choose a reason for hiding this comment

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

(suggestion): we should also add that we use (create it locally first) a default.local.toml in merino/configs which is used by the locally running instance of merino via make dev.

default.local.toml is where we store the actual api keys for vendors like accuweather / polygon e.t.c to test against the live endpoints from our local machine. Configs in this file will override configs from default.toml. And, this config file is in the .gitignore so it'll prevent you from committing secrets 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, Cool! I didn't know about that. Thanks!

@jrconlin jrconlin marked this pull request as draft August 27, 2025 20:22
@jrconlin
Copy link
Member Author

Crap, just saw I had marked this ready for review. It's not. Reverting to draft.

Add the Sports Data suggestion provider, as well as the merino data
injestion job.

Closes: DISCO-3666
@jrconlin
Copy link
Member Author

Ok, post sports updates added, and a bit of clean-up / simplification.

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.

4 participants