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

Can't mike deploy when using !relative in mkdocs config #199

Closed
GabDug opened this issue Jan 24, 2024 · 11 comments
Closed

Can't mike deploy when using !relative in mkdocs config #199

GabDug opened this issue Jan 24, 2024 · 11 comments

Comments

@GabDug
Copy link

GabDug commented Jan 24, 2024

Summary

I use !relative tags in my mkdocs.yml.

While mike serve works fine, mike deploy ... will show an error:

error: MkDocs encountered an error parsing the configuration file: could not determine a constructor for the tag '!relative'
  in "mkdocs.yml", line 32, column 15

Configuration and Logs

  • Repository:
  • Platform: mac OS
  • mike version: 2.0.0

Steps to Reproduce

  1. Have a !relative tag in your mkdocs configuration (e.g. https://github.com/ManoManoTech/firefighter-incident/blob/main/mkdocs.yml)
  2. Run mike deploy v1.2.3

Expected Behavior

Should work, as when running mkdocs build of mike serve

Additional Context

Possible fix:
in mike.mkdocs_utils.inject_plugin, using mkdocs.config.load_config instead of mkdocs.yutils.yaml_load seems to be a valid workaround: config = mkdocs.config.load_config(f).data

I can open a MR if needed.

@jimporter
Copy link
Owner

jimporter commented Feb 6, 2024

@GabDug Unfortunately, I don't think your proposed fix is correct. inject_plugin is designed to produce a new (temporary) mkdocs.yml file with the mike plugin added in if needed. yaml_load just performs the necessary YAML loading, whereas load_config does a lot of extra manipulation of the YAML data structure. For example, suppose I start with this mkdocs.yml:

site_name: test
docs_dir: docs

nav:
  - Home: index.md

Using yaml_load will give me a YAML data structure with just that data in it. load_config gives this:

config_file_path: <test>/mkdocs.yml
copyright: null
dev_addr: !!python/object/new:mkdocs.config.config_options._IpAddressValue
- 127.0.0.1
- 8000
docs_dir: <test>/docs
edit_uri: null
edit_uri_template: null
exclude_docs: null
extra: !!python/object:mkdocs.config.base.LegacyConfig
  _Config__user_configs:
  - {}
  _schema: !!python/tuple []
  _schema_keys: !!set {}
  config_file_path: <test>/mkdocs.yml
  data: {}
extra_css: []
extra_javascript: []
extra_templates: []
google_analytics: null
hooks: {}
markdown_extensions:
- toc
- tables
- fenced_code
mdx_configs: {}
nav:
- Home: index.md
not_in_nav: null
pages: null
plugins: !!python/object/new:mkdocs.plugins.PluginCollection
  dictitems:
    search: &id001 !!python/object:mkdocs.contrib.search.SearchPlugin
      config: !!python/object:mkdocs.contrib.search._PluginConfig
        _Config__user_configs:
        - {}
        _schema_keys: !!set
          indexing: null
          lang: null
          min_search_length: null
          prebuild_index: null
          separator: null
        config_file_path: <test>/mkdocs.yml
        data:
          indexing: full
          lang: null
          min_search_length: 3
          prebuild_index: false
          separator: '[\s\-]+'
  state:
    events:
      build_error: []
      config:
      - !!python/object/apply:builtins.getattr
        - *id001
        - on_config
      env: []
      files: []
      nav: []
      page_content: []
      page_context:
      - !!python/object/apply:builtins.getattr
        - *id001
        - on_page_context
      page_markdown: []
      page_read_source: []
      post_build:
      - !!python/object/apply:builtins.getattr
        - *id001
        - on_post_build
      post_page: []
      post_template: []
      pre_build:
      - !!python/object/apply:builtins.getattr
        - *id001
        - on_pre_build
      pre_page: []
      pre_template: []
      serve: []
      shutdown: []
      startup: []
      template_context: []
remote_branch: gh-pages
remote_name: origin
repo_name: null
repo_url: null
site_author: null
site_description: null
site_dir: <test>/site
site_name: test
site_url: null
strict: false
theme: !!python/object:mkdocs.theme.Theme
  _custom_dir: null
  _vars:
    analytics:
      gtag: null
    highlightjs: true
    hljs_languages: []
    hljs_style: github
    include_search_page: false
    locale: !!python/object:babel.core.Locale
      _Locale__data: null
      language: en
      modifier: null
      script: null
      territory: null
      variant: null
    name: mkdocs
    nav_style: primary
    navigation_depth: 2
    search_index_only: false
    shortcuts:
      help: 191
      next: 78
      previous: 80
      search: 83
  dirs:
  - <...>/site-packages/mkdocs/themes/mkdocs
  - <...>/site-packages/mkdocs/templates
  name: mkdocs
  static_templates: !!set
    404.html: null
    sitemap.xml: null
use_directory_urls: true
validation: !!python/object:mkdocs.config.defaults.Validation
  _Config__user_configs:
  - {}
  _schema_keys: !!set
    links: null
    nav: null
  config_file_path: <test>/mkdocs.yml
  data:
    links: !!python/object:mkdocs.config.defaults.LinksValidation
      _Config__user_configs:
      - {}
      _schema_keys: !!set
        absolute_links: null
        not_found: null
        unrecognized_links: null
      config_file_path: <test>/mkdocs.yml
      data:
        absolute_links: 20
        not_found: 30
        unrecognized_links: 20
    nav: !!python/object:mkdocs.config.defaults.NavValidation
      _Config__user_configs:
      - {}
      _schema_keys: !!set
        absolute_links: null
        not_found: null
        omitted_files: null
      config_file_path: <test>/mkdocs.yml
      data:
        absolute_links: 20
        not_found: 30
        omitted_files: 20
watch: []

The goal here is to produce a temp mkdocs.yml that MkDocs can reload. While that monstrosity above might work, I'd strongly prefer a solution that keeps the number of transformations on that file to a bare minimum. In conclusion, I think it'd be better if MkDocs upstream fixed this by providing a public API for "give me the YAML config in a form that I can modify and reserialize to disk". If the MkDocs maintainers refuse to do that for some reason then I suppose I could add a workaround to mike, but this seems like a case where MkDocs is in a better position to provide this code. Could you file this with MkDocs?

Closing this for now, but we can reopen it depending on the MkDocs maintainers' response.

@jimporter jimporter closed this as not planned Won't fix, can't repro, duplicate, stale Feb 6, 2024
@GabDug
Copy link
Author

GabDug commented Feb 18, 2024

Hey! Sorry for the late response!

I understand your concern of doing too many transformations. An alternative could be to keep the yaml_load function, but pass it a a Mkdocs Yaml Loader. If the Loader has a configuration, it will resolve the !relative tags.

E.g.:

@contextmanager
def inject_plugin(config_file):
    with _open_config(config_file) as f:
        mkdocs_cfg = mkdocs.config.load_config(f)
    with _open_config(config_file) as f:
        config_file = f.name
        config = mkdocs.utils.yaml_load(f, loader=get_yaml_loader(config=mkdocs_cfg))
...

Here the config objet only has keys present in the yaml file, but with the !relative tags resolved.

With this setup, a MR to mkdocs is not required, and we do not add custom code.

What do you think?

@GabDug
Copy link
Author

GabDug commented Feb 18, 2024

Side question @jimporter -- would you be interested in some help to add type hints? I guess it would help readability, especially for new contributors :)

@jimporter
Copy link
Owner

jimporter commented Feb 18, 2024

What do you think?

I'd prefer the MkDocs developers exposed an API for this. If they refused, then I suppose something along these lines would be ok.

Side question @jimporter -- would you be interested in some help to add type hints? I guess it would help readability, especially for new contributors :)

Hmm, I don't think so, but thanks for the offer. I write a lot of "typed Python" at my day job and the limitations of the type annotations frustrate me enough that I routinely fantasize about throwing it all away and changing languages. (This is made all the worse for mike since I try to remain compatible with older Python versions, and the type annotations in those versions are even more limited.)

@nardew
Copy link

nardew commented Apr 1, 2024

Hi, I have the same issue. What is the solution?

@jimporter
Copy link
Owner

Hi, I have the same issue. What is the solution?

Currently, there is none. However, I think I've found a trick to round-trip YAML tags (including !relative), so it should be possible for me to write a patch to support this.

@jimporter jimporter reopened this Apr 3, 2024
@jimporter
Copy link
Owner

@GabDug @nardew If you could try the latest commit out and let me know if it works for you, that would help. If it all looks ok, I'll probably cut a new release of mike in a week or so.

@GabDug
Copy link
Author

GabDug commented Apr 4, 2024

@jimporter I haven't took much time to check so it's to take with a grain of salt, but it looks like there's another error.
From

error: MkDocs encountered an error parsing the configuration file: could not determine a constructor for the tag '!relative'

to

error: MkDocs encountered an error parsing the configuration file: could not determine a constructor for the tag 'tag:yaml.org,2002:python/name:material.extensions.emoji.twemoji'
  in "firefighter-incident/mkdocs.yml", line 162, column 18

beause of

  pymdownx.emoji:
    emoji_index: !!python/name:material.extensions.emoji.twemoji
    emoji_generator: !!python/name:material.extensions.emoji.to_svg

I will try to double check and give you a way to reproduce in a few days.

GabDug added a commit to ManoManoTech/firefighter-incident that referenced this issue Apr 4, 2024
GabDug added a commit to ManoManoTech/firefighter-incident that referenced this issue Apr 4, 2024
@jimporter
Copy link
Owner

Oh, gross... I forgot that MkDocs lets people inject arbitrary Python objects into mkdocs.yml. While I'm not a fan of that, I suppose mike needs to support what MkDocs supports here. I pushed a fix for that if you want to try it out.

GabDug added a commit to ManoManoTech/firefighter-incident that referenced this issue Apr 4, 2024
@GabDug
Copy link
Author

GabDug commented Apr 4, 2024

Seem to work at first glance but I notice some issues, that may or may not be related:

  • The CI started to fail
  • In another project, with a mkdocs.yaml that inherits from the one with the !python tags using INHERIT: source/mkdocs.yml, I get error: MkDocs encountered an error parsing the configuration file: could not determine a constructor for the tag 'tag:yaml.org,2002:python/name:material.extensions.emoji.twemoji' in "source/mkdocs.yml", line 162, column 18. This happens only with mike deploy, not mkdocs build.

Will investigate later!

@nardew
Copy link

nardew commented Apr 5, 2024

@jimporter I confirm it works for me. Thank you!

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

No branches or pull requests

3 participants