Namespace-based Export Plugins

[minchinweb]

I'm thinking about reworking the export plugin system to be auto-discovered based on the module name. Assuming this works well, it could be expanded to import plugins and possibly the storage backends as well.

There would be two namespaces: one for internal plugins, and another for external/contributed plugins.

The usecase would be to solve issues such as #779 where there is a difference of opinion of what the export should look like, or if you have a special format that you need. "Contributed" plugins could also be a way to test out a format before added in to the core of jrnl.

For myself, I have a "special" plugin that I used as a prep step to feed my jrnl entries into Pelican (a static site generator). It's always seemed like too "special" of a case to submit for the main repo, but it's something I use regularly. With the support and bug fixes to DayOne support, this is probably the only reason I have left to keep a private fork of jrnl.

In terms of implementation, I'm imagining that each plugin would have five methods:

• one that returns it's name (e.g. "Markdown Exporter")
• one that returns it's version (e.g. "2.4.4-beta"; for built-in plugins, this would match jrnl)
• one that returns a list of "format codes" that it will process (e.g. ["md", "markdown"]; this is was will used by the export command of jrnl)
• one that is passed a single entry and returns the exported entry
• one that is passed a journal and returns the exported journal

I expect that contributed plugins would override the build-in plugins if they defined the same format code.

By using namespaced plugins, the plugin would be install-able via pip and would require no more configuration or activation beyond being importable by Python.

I haven't written any code yet (but I've been trying to figure out how to do this for probably 3 years...), but I wanted to get some thoughts on if there was support for this before I spend a bunch of time on it.

Thanks,

[wren]

Thanks for bringing this up; I've also been wanting to get our plugins re-done.

I'm still thinking through this, but here are my thoughts so far.

• Our current system doesn't mesh well with distribution methods. This is due to the fact that plugins are expected to be loaded from a directory the jrnl core. This works just fine if you're running jrnl from source, but anywhere else (pipx, brew, pip, etc) there's a decent chance your plugin will be uninstalled on upgrade. The best way to deal with this at the moment is to either reinstall your plugin (copy the file into the correct folder in jrnl) after each upgrade, or maintain a private fork (as you mentioned) where you basically have a patch for your plugin on top of the jrnl source. Both are kind pretty annoying, and users really shouldn't have to deal with either of them.

• Significantly changing the way plugins work is a breaking change, and seems like a major change in increment semantic versioning terms (meaning we update the version to v3.0.0). In and of itself that's not a bad thing, and I'm very okay with doing it, but I do want to be sure we think through the changes we want, and that we're "getting our money's worth" for a major change.

• The current plugins can't really do very much other than handle formatting, and aren't hugely different from piping the data to an external program that does the same thing. I would love for our plugin system to allow for better flow control, and even creating new journal types.

• I don't think the jrnl core should support very many journal types, but having a plugin system that can handle that would allow the community to create as many different types as it needs, without needing to do any of the above jankyness to keep their plugins installed.

• This kind of system would definitely be an increase in scope from what you are discussing in the OP. What would be the best way to split this work up in a way that benefits users more quickly so that some big jrnl v3 release isn't forever on the horizon?

• Can we fix the first point up top (about reinstalling plugins on upgrade) be fixed without breaking changes to the current plugin system? Maybe something along the lines of a user-defined directory (maybe in config) to load plugins from in addition to the jrnl core plugins? This solve an immediate problem for users while buying us more time to implement the more in-depth changes to the plugins.

• How do hooks (Application hooks #921) factor into this (if at all)? Could implementing hooks (maybe in conjunction with a user plugin directory?) help us solve some of those same more immediate problems?

That's all I've got for now. Sorry it's disorganized, but I'm still mulling over all of it.

[wren]

Oh, I forgot to mention pluggy. It looks like a very useful, lightweight library for managing this type of problem. We'd just have to design a spec for what we wanted, and it can handle pretty much the rest of it.

[wren]

I'm kinda liking the approach of moving the plugin directory (so a user can just drop in their plugins independent of the jrnl core plugins). This would buy us some time to redesign the plugin system while not breaking backwards compatibility (for now).

What do you think, @minchinweb?

[minchinweb]

Hey @wren ! I think your vision is fairly close to mine, so I'll start working on something!

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[minchinweb]

still planning on making this happen!

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[minchinweb]

not stale...

[wren]

@minchinweb How's this going? Any progress? Is there anything we can help with? I'd love to get the solution we talked about in to jrnl soon, since it'll make it easier to make updates to the exporters without worrying about breaking them for users.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[minchinweb]

bump

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[minchinweb]

not stale, still WIP

[wren]

Oh, ha. It looks like stalebot is working again. It had gone down for a while and I honestly wasn't sure if they were ever going to fix it.

[micahellison]

This code is currently in the merged PR in the feature-plugins branch. There's a little work left to do before merging it in:

• plugins listed in jrnl --version should be in jrnl --diagnostic
• deal with folder-changing issue by removing the two init.py files from jrnl and moving their functionality elsewhere (I think @wren has this stashed somewhere)
• documentation changes
  • since the plugin documentation is for developers, I want to unify it w/ developer documentation. Seems like a good opportunity to get Move and expand contributing.md content into docs site #1180 done
  • make the documentation tone and form consistent
• simplify the CI/testing workflows - ideally it should use poetry, and the makefile should support it

There's one more thing that I want to change, though it's not a showstopper: ideally, the feature file should be able to use a "Given" step to install and test plugins. When we start getting issues from plugin developers, this will make it a lot easier to prevent regressions.

[minchinweb]

But -- and maybe I'm reading your suggestion wrong -- wouldn't putting import jrnl anywhere inside the jrnl module itself cause a circular dependency?

Surprisingly no. Maybe this is because it's an explicit import? The one place you can't put it is in the __version__.py file itself, as somewhere after that file is imported, the reference is still to the module. So taking it a step further, #1296 just overrides the module!

I agree we don't want third-party plugins hooking into the jrnl version. In that case, wouldn't we want to avoid showing them how to do it in the first place?

I suppose my logic was that by showing them that a version is defined, they'll know they need to provide one. But a guess someone new could draw either conclusion.

We could also leave the version as is and just not do anything with it (like in the diagnostic page we could just leave version out for core plugins).

Do we also leave out the import paths?

[wren]

Hi, @minchinweb!

I just wanted to follow up on the status of this. I know there are some questions that I need to answer, so I wanted to be sure you have all the info from me you need.

With that in mind, I wanted to say a few things.

First, thank you for all the hard work on this feature. I know it's important to you, and it's good to see a contributor step up to add features they want to see.

That being said, it looks like the plugins branch still needs some work before it's up to the quality I'd like to see in jrnl. Don't get me wrong; I don't want to minimize the progress, and I recognize there's already been a lot of work that's gone into it. It's just that it's a big project, and it requires a lot of hours of work.

Speaking only for myself: this feature isn't at the top of my priority list. And as such, I feel like it's been taking up a bit too much of my time lately (I'd like to focus my efforts on fixing bugs that have piled up). That's not to say that I won't happily accept PRs to keep this moving, just that I won't be working on those PRs myself for now.

For the items remaining, I think the PR comments so far should be clear in what tasks are remaining, but if there's any confusion whatsoever, please let me know and I'll do what I can to lay out a clear path to success.

Thanks again, for all the hard work, and thank you for your understanding in this.

[minchinweb]

Hi @wren ; thanks for the update.

The plugins got merged to a feature branch (see #1216), and so I lost my main point of tracking what was outstanding. I will try and add everything outstanding here, let me know if I've missed anything.

• "plugins listed in jrnl --version should be in jrnl --diagnostic" (source) -- I created a Pull Request (see Plugin list on Diagnostic #1282), but is waiting on a decision on your part. Personally, I would have the internal plugins report jrnl's version number
• deal with folder-changing issue by removing the two __init__.py files from jrnl and moving their functionality elsewhere (I think @wren has this stashed somewhere) (source) -- has this been applied? Again, I think it's in your court.
• documentation changes (source) -- I included documentation that I thought would be useful/needed. @micahellison has one Work-In-Progress Pull Requests ([WIP] Edit feature plugin documentation #1265) and one somewhat related issue (Move and expand contributing.md content into docs site #1180), but it's not entirely clear (to me) what needs to change/what is holding up merging the plugins.
• my pull request Custom importer Improvements #1281 remains open. Do the tests just need to be moved to the new framework?

Is that everything, or have I missed something?

[wren]

Ok, thanks for letting me know. I'll post each thing below as a separate post so we can use threading* to keep the discussion organized.

• Hopefully! This is the first time trying this on Github Discussions, so we'll see how it works out.

[minchinweb]

Thanks @wren! It will take me a bit to work through the list and a bit more to push code here. The discussion (and its threading) seems like a reasonable way to keep everything in one place!

[wren]

• "plugins listed in jrnl --version should be in jrnl --diagnostic" (source)

[wren]

MinchinWeb:

I created a Pull Request (see Plugin list on Diagnostic #1282), but is waiting on a decision on your part. Personally, I would have the internal plugins report jrnl's version number

[wren]

I think that the output of diagnostic should be as useful as possible, since it's what we ask people to paste in when they file an issue. To me, this means that we need to reduce noise, and a big part of that is:

• Only output relevant info
• Only output that info once
• If something never changes, then there's no reason to output it

So, here's the current output of the plugin list:

Active Plugins:
    Importers:
        jrnl : v2.8.1 from jrnl.plugins.importer.jrnl
    Exporters:
        boxed : v2.8.1 from jrnl.plugins.exporter.fancy
        dates : v2.8.1 from jrnl.plugins.exporter.dates
        default : v2.8.1 from jrnl.plugins.exporter.pretty
        fancy : v2.8.1 from jrnl.plugins.exporter.fancy
        json : v2.8.1 from jrnl.plugins.exporter.json
        markdown : v2.8.1 from jrnl.plugins.exporter.markdown
        md : v2.8.1 from jrnl.plugins.exporter.markdown
        pretty : v2.8.1 from jrnl.plugins.exporter.pretty
        short : v2.8.1 from jrnl.plugins.exporter.short
        tags : v2.8.1 from jrnl.plugins.exporter.tag
        text : v2.8.1 from jrnl.plugins.exporter.text
        txt : v2.8.1 from jrnl.plugins.exporter.text
        xml : v2.8.1 from jrnl.plugins.exporter.xml
        yaml : v2.8.1 from jrnl.plugins.exporter.yaml

This list has a lot of repeated info, and it's difficult to see relevant info. I'd like to see something more like this:

Active Plugins:
    Internal:
        Import:
            - jrnl (jrnl)
        Export:
            - plugin_name (format_name1, format_name2, etc)
            - fancy (fancy, boxed)
            - dates (dates)
            - pretty (default, pretty)
            - markdown (markdown, md)
            - ...

    External:
        Import:
            - plugin_name version_number (format_name1, format_name2, etc)
            - foo v8.1.5 (foo, bar)
        Export:
            - baz v8.1.5 (baz)

[minchinweb]

I can reformat the output.

I big part of why I wrote this the way it is was to understand what import/export formats are available and see right away which plugin was providing the format I was interested in. This is partly because you can overwrite one (but not all) of a build in plugin's formats. (i.e. I cared more about a specific format than the specific plugins.)

What about something like?

Active Plugins:
    Exporters:
        boxed : [internal]
        default : v1.1.0 from jrnl.plugins.contributed.custom-default

I think part of listing the full import path for custom plugins in that there is no requirement that the actual plugin code be in the jrnl.plugins.contrib namespace, only the "registration" code live there.

[wren]

That's a very good point. Thank you for bringing it up!

I like your suggested output, and just have very minor changes.

Formats:
    Import:
        jrnl    : jrnl (internal)
    Export:
        boxed   : fancy (internal)
        default : custom-default v1.1.0 from foo.bar.somewhere_else
        foo     : bar v1.3

The changes being:

1. "Formats", "Import, and "Export in the titles so it's more consistent with the docs
2. Include the name of the plugin even if it's internal
3. Only include the `from foo.bar.whatever` if the plugin is not at the default location (so there's less noise for the defaults)
4. Line up the colons so it's more consistent with other general output

[minchinweb]

@wren #1282 has been updated to this format and is ready for merging.

[wren]

• deal with folder-changing issue by removing the two __init__.py files from jrnl and moving their functionality elsewhere (source)

[wren]

micahellison:

(I think @wren has this stashed somewhere)

[wren]

MinchinWeb:

has this been applied? Again, I think it's in your court.

[wren]

I'm not 100% sure what this was referring to, but all of the __init__.py files are gone. So, I think this one is already done. Is that right?

[minchinweb]

I think it was resolved, but it was never reported in the previous thread, so I included it here for completeness. If you think it's done, check it off!

[wren]

• documentation changes (source)

[wren]

MinchinWeb:

I included documentation that I thought would be useful/needed. @micahellison has one Work-In-Progress Pull Requests ([WIP] Edit feature plugin documentation #1265) and one somewhat related issue (Move and expand contributing.md content into docs site #1180), but it's not entirely clear (to me) what needs to change/what is holding up merging the plugins.

[wren]

cc: #1267

[micahellison]

I think I'll be able to cover the docs changes, once the feature_plugins branch is stable again.

[minchinweb]

Thanks for tackling that @micahellison !

[wren]

MinchinWeb:

my pull request Custom importer Improvements #1281 remains open. Do the tests just need to be moved to the new framework?

[wren]

From the last comments on the PR, it looks like the tests for this aren't running and/or passing. So, it should be updated to the latest and get the tests passing. I'll try to keep PR-specific conversation in the PR itself, though.

[minchinweb]

I will look further into it. One issue (I believe) was that the Windows tests weren't running at all, and I don't think that was related to my code...

[wren]

I think the tests are stable on Windows now, too, but I'm not sure.

@micahellison You run windows, right? Are the tests running for you?

[micahellison]

Let's see... on feature-plugins, I see a linting issue that's causing make test to fail for me. And when I run poetry run pytest I get an error that indicates that the plugin code has broken the tests for the fancy exporter, hopefully just a renaming issue. I don't think either of those issues are Windows-specific, though.

Still, there could be issues in CI even after resolving those due to this caching issue we found today. If the tests are still failing after you've resolved them locally, feel free to @ me and I'll bust the cache.

[wren]

From above:

• Significantly changing the way plugins work is a breaking change, and seems like a major change in increment semantic versioning terms (meaning we update the version to v3.0.0). In and of itself that's not a bad thing, and I'm very okay with doing it, but I do want to be sure we think through the changes we want, and that we're "getting our money's worth" for a major change.

[wren]

• Can we fix the first point up top (about reinstalling plugins on upgrade) be fixed without breaking changes to the current plugin system? Maybe something along the lines of a user-defined directory (maybe in config) to load plugins from in addition to the jrnl core plugins? This solve an immediate problem for users while buying us more time to implement the more in-depth changes to the plugins.

[wren]

From above:

I'm kinda liking the approach of moving the plugin directory (so a user can just drop in their plugins independent of the jrnl core plugins). This would buy us some time to redesign the plugin system while not breaking backwards compatibility (for now).

What do you think, @minchinweb?

[micahellison]

I don't really see this as a "breaking change" because the current "plugin" system involves overwriting jrnl's code in source, and it's not documented anywhere. There's no promise being made that any plugin work you do now will be supported in the future (and, in fact, if you use a package manager, your change will be overwritten when you upgrade anyway).

However, this new plugin system can easily lead to many breaking changes. With this new system, we're exposing and documenting the internals of jrnl's classes as a sort of API for plugin developers. After releasing it, any reworking that we do on these classes carries the risk of becoming a breaking change for existing plugins, maybe even without our knowledge. Are those all major releases? Or do we add a caveat emptor in the docs to tell plugin developers that these classes are subject to change and may break their plugins even on minor releases?

[wren]

That's a good point. As long as we keep the functionality of the plugins themselves identical to the current version, we can put this new plugin system around it without breaking any current functionality.

That's a good point about future expansion, though. What do future plugins look like in this system? If we add an encryption for example, what namespace does that use? Or what if we change a method signature or something? How does this system handle that?

As mentioned, adding this isn't a breaking change, but changing it later will be a breaking change. So, we should try to make sure we're considering the future for this.

[minchinweb]

Adding plugins that deal with on-disk formats or encryption or other "hooks" is beyond what I'm trying to do here. But in general, the file/code layout I'm using could easily be extended to support other types of plugins For example, backend/storage plugins could be at jrnl/{plugins, contrib}/storage/*.py.

That said, what I've done here is a pretty limited version of "plugin": these plugins are expected to have one or two specific functions that can be called from the main program flow; they're not designed to change jrnl into a fundamentally different program.

---

As for versioning, I don't think this (the import/export plugins as proposed) constitute a "breaking change", as this doesn't change anything that is part of the pubic/external API (i.e. there has been no official way to add import or export formats to date). If this is a "breaking change", it's because this drops that template exporter (which I feel slightly bad about, but I never quite understood :( ).

Although maybe it makes sense to bump the major version number from a marketing perspective.

As for version numbers going forward, if we intentionally break/change the import/export API (or the Entry or Journal classes that they internally rely on), then that should be considered a "breaking change". But I've also tried to keep the API simple to try to minimize the expected chances of that happening. If there is an accidental breakage, it will probably have to be handled as it happens: if it was on purpose, bump the major number and release a "old line" bugfix release reversing the change? I think generally we can rely on the test suite here; perhaps the warning we add to the documentation is that if the developer is relying on some functionality not covered by the test suite, we won't know if we broke it so please submit a PR expanding the test coverage.

[wren]

From above:

• How do hooks (Application hooks #921) factor into this (if at all)?
• Could implementing hooks (maybe in conjunction with a user plugin directory?) help us solve some of those same more immediate problems?

[wren]

I think this question feel through the cracks in the original discussion. I just put it here to make sure we don't forget about it.

[wren]

• import jrnl.__version__ doesn't currently work (have to do import jrnl.__version__.__version)

[wren]

Yeah, I'm not jazzed about it either. Not sure if that's doable without the init files, though. Maybe we can move to something better like jrnl.constants or something?

[wren]

MinchinWeb:

I can make jrnl.__version__ work, the code is simple; I'm just not sure where to put the code:

import jrnl

setattr(jrnl, "__version__", "2.8.0")

[wren]

@minchinweb Is there anywhere we can put your suggestion that won't cause a circular dependency? I don't see how we can import jrnl inside of jrnl without problems.

[minchinweb]

Is there anywhere we can put your suggestion that won't cause a circular dependency?

@wren Without __init__.py files, there is no code run on import so you can't have a circular import (i.e. import jrnl doesn't import anything in turn).

But in general, the import jnrl line needs to be there because we need the jrnl namespace to exist for Python before we can reference it to change it.

In any case, this code was already been merged in, see #1296 , so this can be marked as resolved?

[wren]

@minchinweb You're absolutely right! I completely forgot about the import sys from the PR. Yup, this one is good to go.

[wren]

• Empty dirs in jrnl/contrib/*

[minchinweb]

These are needed so that the namespace imports work (for the user plugins).

[wren]

Having empty files and to maintain empty directories without which the app will break is an anti-pattern in my eyes. Is there a way to get rid of them? Can we maybe replace the these dirs with a try block around the imports?

[wren]

Tests:

• Move to pytest
• Create given step for whether the sample plugin is installed or not (instead of re-running the test suite based on tags)
• Don't rely on specific plugin to be installed to know when plugins exist (e.g. flag.py)

[wren]

Sample plugins (from tests/external_plugins_src/):

• Update README.md to explain all plugins (currently only mentions some)
• Add examples of formatting for what each plugin expects
• Update setup.py to include license (I'm pretty sure this defaults to whatever license jrnl has since it's in the same repo, but I'd prefer to have that be explicitly set by the author)
• Optional, but I think very useful: update json importer to match format with our current json exporter (this would allow us to write some more thorough tests for these plugins)

[jaxspades]

Hi, this looks awesome, and I love the idea of jrnl so far, as I want to move from Day One to something I have more control over and can run locally, but I think I would need more plugin support to do so. Is this feature for plugins still in active development? Is there anything another python developer could help with to finish this? Thank you for everything so far, jrnl looks great, and this feature looks great too from looking over the branch!

[wren]

@jaxspades We haven't heard from the contributor that was working on this in a while, so my assumption is that it's no longer in active development. As for what a python dev can do to help finish this, the discussion above is basically a list of todo items that remain to finish this feature. The only todo item I would add is that it'll need to get the develop branch merged in in order to get caught up with the work done since this was last in development.

From what I remember, the work was almost done, and probably doesn't have that much left to do in it. If you decide start working on this, please feel free to ask any questions that come up.

[jaxspades]

I will look over the code and tasks, see if this is something I can take on. I am looking to be able to display images and videos and files like documents and pdfs in jrnl, and didn't see if that's possible, so my thought was the plugin system would help with that for displaying everything in that static site output.

[minchinweb]

@wren The reason you haven't heard much from me is you told me to go away, so I have.
As for the codebase, it was in a working state when I last worked on it. If it no longer works, that would most likely be due to change in jrnl. The remaining issues are administrative, which I am not able to solve.

[wren]

Hi @minchinweb, I hope you're doing well. I'm sorry if my earlier comment came across as "go away." That's not how I meant it. I value and appreciate your time, so I want to be sure you're taking care of yourself (we all need breaks every now and then).

I said it before, and I still mean it: we'll be here whenever you're ready to come back and continue contributing.