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

Remove duplicate netcdf entry (triggers "Type application/netcdf is already registered as a variant of application/netcdf.") #50

Closed
wants to merge 2 commits into from

Conversation

shioyama
Copy link

@shioyama shioyama commented Jan 17, 2022

This seems to be a duplicate entry. We've encountered an issue where the second entry triggers a warning here:

https://github.com/mime-types/ruby-mime-types/blob/5b931d70d74f3d1ba25babac03237ae450ca5c5b/lib/mime/types.rb#L189-L193

    if !quiet && @type_variants[type.simplified].include?(type)
      MIME::Types.logger.warn <<-WARNING.chomp.strip
        Type #{type} is already registered as a variant of #{type.simplified}.
      WARNING
    end

This emits Type application/netcdf is already registered as a variant of application/netcdf.

I've just removed the first of the two. The change traces back to 8c995e61 released in v3.2022.0105.


This change is Reviewable

@shioyama shioyama changed the title Remove duplicate netcdf entry Remove duplicate netcdf entry (triggers "Type application/netcdf is already registered as a variant of application/netcdf.") Jan 17, 2022
@halostatue
Copy link
Member

Please check types/application.yml and types/provisional-standard-types.yml and modify those. Simply modifying the data/mime.content_type.column file will break everything (there are 8 column files). The files in data/ are generated files and should not be modified manually. The source will be in one or both of the above files.

@shioyama
Copy link
Author

@halostatue I've updated types/provisional-standard-types.yml and run rake convert:yaml:json and rake convert:yaml:columnar to generate data files.

@halostatue
Copy link
Member

In general, PRs should just be for the YAML files; I do the conversion (there are three conversions now) as part of the release process. It’s not a big deal.

@halostatue
Copy link
Member

So I’ve done a quick bit of checking, and the story isn’t great at this point.

Removing the type from the provisional file won’t do any good, because there’s a provisional registration with IANA, so the next run of rake mime:iana will restore the provisional record.

Removing it from types/application.yml isn’t great either, because while it’s not registered, it does have some extensions recorded:

- !ruby/object:MIME::Type
  content-type: application/netcdf
  encoding: base64
  extensions:
  - nc
  - cdf
  registered: false

I deliberately do not permit the customization of the provisional file, and we don’t currently overlay the provisional data on the existing types files, so we either:

  1. continue with this warning as is,
  2. remove the unregistered application/netcdf entry from types/application.yml (and lose the extensions), or
  3. change how provisional types are handled from the IANA converter such that they overlay any types in the primary types.

We would need to probably keep the provisional file so that we can see when types are removed from provisional registration without being promoted to official registration (maybe we keep those but mark them as registered: false).

The third option is the correct option, but it is also a lot of code change in the support code because it would now need to deal with promotion and/or removal. I also don’t know how important the extensions are for application/netcdf, so I can’t speak on the harm that option 2 would cause; it could be mitigated by adding an issue wanting the extensions added back once application/netcdf is a properly registered (and not merely provisional) type.

Unfortunately, the means that this PR cannot be merged as is. I’m not closing it yet, as we need to figure out the correct resolution.

@shioyama
Copy link
Author

Thanks for the very detailed response! The warning may confuse people, but it's not urgent. We've opted to just add a line that ignores it, which works for us until the issue is resolved.

I'll let you decide if/when to close this, and thanks again for the explanation.

@noff
Copy link

noff commented Mar 7, 2022

Any updates on this issue? The warning is quite annoying due to it outputs the message to STDOUT and cron MAILTO command delivers it as output of the cron task.

@halostatue
Copy link
Member

Not at the moment, sorry. It’s not a trivial issue to resolve. If you’d like to open an issue on mime-types/ruby-mime-types, I can look at ways that this message can be suppressed (it won’t be removed, but I could suppress it either with an environment variable or by checking a common environment variable like RAILS_ENV), but I have an extremely busy schedule this week so it will probably not be looked at until the weekend or next week.

@dogweather
Copy link

dogweather commented Jan 23, 2023

IMO the problem is, the library shows a warning message to the wrong audience (application developers). It's not understandable to app devs, not marked with its provenance (source lib name), and finally, app devs have no way to resolve it.

Instead, the message seems aimed at mime-types-data library developers. So, what's the shortest path to changing the log level (DEBUG ?) and/or logic to show it only to the correct audiences?

I submitted a PR to make this DEBUG level: mime-types/ruby-mime-types#170

@halostatue
Copy link
Member

IMO the problem is, the library shows a warning message to the wrong audience (application developers). It's not understandable to app devs, not marked with its provenance (source lib name), and finally, app devs have no way to resolve it.

This is incorrect. The message is to developers who use mime-types. Yes, a lot of people only use mime-types for the associated data (mime-types-data), but it is perfectly extensible, and this warning is completely correct in that case.

@dogweather
Copy link

Yes, a lot of people only use mime-types for the associated data (mime-types-data)...

How can those people be better supported?

@halostatue
Copy link
Member

Yes, a lot of people only use mime-types for the associated data (mime-types-data)...
How can those people be better supported?

I don’t have an answer for that—as the lack of resolution to this PR and other possible fixes shows. It is problematic when multiple matching types are added, so the warning isn’t inappropriate. There’s no way to say "ignore any warnings while loading this, but then start showing warnings after this is loaded". Even if there were, I’m not sure whether that would be the right thing to do, because there’s a more fundamental issue of duplicate data in this database, and it appears to be unavoidable with the design constraints currently present.

@dave105010
Copy link

dave105010 commented Feb 16, 2023

This situation is creating a lot of clutter in our logs. It started appearing after Rails 7 upgrade. This also affects our cron scripts that run periodically. All of them are sending the warning output and notifying us. Is there a way to get rid of this warning?

@halostatue
Copy link
Member

I have addressed this properly in #53 and will be releasing a new version of mime-types-data tonight that resolves this issue by merging provisional types into the main types and not handling them separately.

Thanks for your contribution, @shioyama and my apologies for how long it took to resolve this.

@dave105010
Copy link

dave105010 commented Feb 18, 2023 via email

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