[taspelund]

Use the older version of the PortFec type when parsing xcvr_defaults.csv then convert to the newer type. This allows us to parse old versions of the file without needing any producers of the file to update them.

[bnaecker]

Are we ever expected to have a situation in which dpd is parsing a file that was built from a different commit?

[taspelund]

Are we ever expected to have a situation in which dpd is parsing a file that was built from a different commit?

I'm not familiar enough with all the different dendrite deployments (e.g. in mfg) to answer that. This seemed like the conservative approach that would allow other suppliers of the CSV file (if they exist) to keep working.

I'm happy to modify the CSV to use the new format, but I didn't want to cause breakages later for components I wasn't aware of.

[bnaecker]

This seemed like the conservative approach that would allow other suppliers of the CSV file (if they exist) to keep working.

I feel like that's more risky than conservative. As an example, if we happened to publish an Omicron release with main, we'd be hosed when trying to load this file! Do we actually know when this was introduced and whether we have this in the wild anywhere?

IMO, we should not do this, and instead enforce that the file and parsing code are always in-sync. Another option, possibly safer, is to make the defaults a static somewhere in the code. That avoids any parsing issues, at the cost of losing the flexibility to drop in a new file as an escape hatch. We've never needed that, while we do have an example of a code / file mismatch. So I'm not sure it's worth keeping that flexibility.

[taspelund]

This seemed like the conservative approach that would allow other suppliers of the CSV file (if they exist) to keep working.

I feel like that's more risky than conservative. As an example, if we happened to publish an Omicron release with main, we'd be hosed when trying to load this file! Do we actually know when this was introduced and whether we have this in the wild anywhere?

IMO, we should not do this, and instead enforce that the file and parsing code are always in-sync. Another option, possibly safer, is to make the defaults a static somewhere in the code. That avoids any parsing issues, at the cost of losing the flexibility to drop in a new file as an escape hatch. We've never needed that, while we do have an example of a code / file mismatch. So I'm not sure it's worth keeping that flexibility.

I'm fine with that approach, and would be happy to update the branch accordingly.

I believe this was introduced quite recently in #418. I'm not sure yet whether this is in the wild. I asked in the mfg matrix channel but haven't heard back yet

[rcgoodfellow]

I've modified this on a few occasions. I think every time it was when a transceiver MPN or MPN variant was missing from the list. But that's always been experimenting for a follow-on PR that fixes the file.

Making these constants seems like a nice simplification. And we can always override them from the CLI or through the API.