Issue 41 #87
Issue 41 #87
Conversation
…rsion 1.6.1 to cnm schema
| click.echo(metgen.banner()) | ||
| configuration = config.configuration(config.config_parser_factory(config_filename), {}) | ||
| metgen.init_logging(configuration) | ||
| metgen.validate(configuration, content_type) |
There was a problem hiding this comment.
May want to insert a validation of the config file? I'm not sure it's necessary, though, because there could be lots of stuff in the config file that we don't care about when we're just validating the json? https://github.com/nsidc/granule-metgen/blob/main/src/nsidc/metgen/cli.py#L54
There was a problem hiding this comment.
My take on the story is that we're focused on validating JSON output in the issue-41 feature, not other things that do in fact need to be validated. I'm inclined to stick with the current setup. What I feel would be better, config-file-wise, is to add the equivalent ofconfig.validate(configuration) to the CLI init processing so that the new .ini file is immediately checked for errors.
|
See what you think about my question above. If you don't think we want to validate the config, I'm good with merging. Looks like I can't approve until that and/or the README conflict is resolve, but I HEREBY APPROVE. |
|
|
||
| `nsidc-metgen` via the MetGenC utility, enables Operations staff and data | ||
| The `MetGenC` toolkit enables Operations staff and data |
There was a problem hiding this comment.
@eigenbeam Here's my argument for just using the term "MetGenC" in this context: The package name nsidc-metgenc includes nsidc for name-spacing purposes. If we want to include nsidc in this README text then I think it should read NSIDC MetGenC (similar to SIPS MetGen). Thoughts?
No description provided.