Custom importer Improvements #1281

minchinweb · 2021-06-27T04:55:00Z

Further to #1006

explicitly write the journal to disk (after importing entries), rather than assuming the plugin will do this
be more consistent in how data fed to the custom JSON importer is dealt with (i.e. pharse both input files and stdin into JSON)
first pass at a test for the custom JSON importer. The test isn't running for me locally, but also isn't displaying any error message, so I'm not sure what's going on

jrnl/plugins/exporter/dates.py

micahellison · 2021-07-03T21:48:18Z

Hey @minchinweb, it looks like the test isn't running because it's being excluded by the @skip_only_with_external_plugins tag. The more I think about it, the more I'm hesitant to ask for a change on this, though, since it would need to be re-written anyway after merging #1193 in, and we're very close to finishing that.

wren · 2021-07-13T19:52:19Z

features/plugins.feature

@@ -31,7 +31,8 @@ Feature: Functionality of Importer and Exporter Plugins
        Given We use the config "basic_onefile.yaml"
        When We run "jrnl --version"
        Then the output should contain pyproject.toml version
-        And The output should contain "<plugin_name> : <version> from jrnl.<source>.<type>.<filename>" 
+        And the output should contain "<plugin_name> : <version> from jrnl.<source>.<type>.<filename>"


It's good that we're fixing up the tests, so this is fine. For future test fixes/updates, though, can we focus on the /tests/bdd/features directory instead?

wren · 2021-07-13T19:53:21Z

features/plugins.feature

+    @skip_only_with_external_plugins
+    Scenario Outline: Custom JSON Import
+        Given we use the config "empty_folder.yaml"
+        When we run "jrnl --import ./features/data/simple_import.json"


No need to fix this here, but when we move this test to Pytest BDD, we should pipe this json in so it sits in the feature file (instead of having a separate file to manage).

wren · 2021-07-13T19:54:09Z

tests/external_plugins_src/jrnl/contrib/importer/simple_json.py

            except KeyboardInterrupt:
                print(
                    "[Entries NOT imported into journal.]",
                    file=sys.stderr,
                )
                sys.exit(0)
+            data = json.loads(f)


wren

I added some more inline comments.

minchinweb · 2021-07-14T03:38:27Z

it looks like the test isn't running because it's being excluded by the @skip_only_with_external_plugins tag.

Except most of the other tests in the file are similarly tagged, and they do run....

As for #1193, it looks like that was merged to develop, so I'm not sure how best to move forward on that.

Otherwise, I think all the review comments have been resolved.

* behavior outline * FIrst pass at allow external plugins * remove template exporter * Add listing of active plugins to '--version' output * Documentation for plugins * [Docs] add custom imports and exporters to site TOC * [Docs] better linewrapping * enforce positive initial linewrap Check column widths update gitignore throw error when linewrap too small simply check for large enough linewrap value * delete unused error message * PR feedback make exception more informative update check_linewrap signature in src and test make check_linewrap a free function * delete unused function * delete else..pass block * newline for make format * Include dates_exporter * Use Base classes for importer and exporters. * [Docs] improve documentation of custom Importers and Exporters * [Testing] separate run with external plugin! * basic behavior test * prototype unittest for JSON Exporter test for unimplemented method * make format delete unused imports * Remove 'importer' or 'exporter' from filenames where not needed * [Test] run different tests with or without the external plugins installed * [Test] move test rot13 plugin into git tree from minchinweb/jrnl-rot13-exporter@0dc912a * consolidate demo plugins to common package * [Docs] name page for plugins * [Docs] include the sample plug in code files directly * style fixes * [test] determine whether to run external plug in tests based on installed packages * improved code documentation * style fixes for GitHub actions * Convert "short" and "pretty" (and "default") formaters to plugins further to jrnl-org#1177 * more code clean up tests pass locally...now for GitHub... * [tests] dynamically determine jrnl version for plugin tests * [GitHub Actions] direct install of testing plugins * Remove template code * [plugins] meta --> collector * [Docs] create scripted entries using an custom importer * (closer to) being able to run behave tests outside project root directory * We already know when exporter to use Don't re-calculate it! * [Tests] don't name test plugin 'testing" If so named, pip won't install it. * [Test] run behave tests with test plugins outside project root * [Test] behave tests pass locally * [Docs] fix typo * [GitHub Actions] run test commands from poetry's shell * black-ify code * [GitHub Actions] move downstream (rather than up) to run tests * [GitHub Actions] set shell to poetry * [GitHub Workflows] Manually activate virtual environment * [GitHub Actions] Skip Windows & Python 3.8 Can't seem to find Python exe? * [GiotHub Actions] explicitly use virtual env * [GitHub Actions] create virutal env directly * [GitHub Actions] better activate of Windows virtual env * [GitHub Actions] create virtual env on Mac * [Github Actions] install wheel and upgrade pip * [GitHub Actions] skip virtual environments altogether * [GitHub Actions] change directory for behave test * Remove Windows exclusions from CI as per note -- they should be working now Co-authored-by: Suhas <[email protected]> Co-authored-by: Micah Jerome Ellison <[email protected]>

Since we're implementing a plugin system that relies on implicit namespacing, we should remove these files so that they don't confuse and muddle our virtual envs (like the ones we use to run tests). Also, they're not longer needed as of Python 3.3. PEP 420 says: Allowing implicit namespace packages means that the requirement to provide an __init__.py file can be dropped completely...

Allows you to access jrnl by running `python -m jrnl`

* Move plugins tests back into normal test workflow * change when tests run * make tests slightly more readable in PR

Importers: handle writing the journal to disk ourselves (rather than delegating to the plugin) Plugins: First pass at testing custom importer

wren · 2021-07-17T20:32:45Z

@minchinweb I updated the feature-plugins branch with the updates to develop, and then resolved the conflicts from this branch (since the merge conflict was pretty big, I didn't want you to have to deal with that). Anyway, thank you for addressing the feedback!

minchinweb · 2021-07-17T20:47:23Z

@wren So what's left? Just to move the tests to the new pytest framework? (P.S. thanks for dealing with the merge conflict!)

…adding in appropriate parameters and no error step

micahellison · 2021-07-17T21:11:38Z

Hey @minchinweb -- I just fixed the test, though it is failing now. There were a couple problems with it -- it was using the scenario outline, but had no examples, so behave iterated through the empty list and ran 0 tests from it. It also wasn't using the --file and --format keywords or checking for errors.

Also, could you modify the import test to take in new data? Currently, it imports data that's already in the journal defined by simple.yaml, so even if the import silently fails, it will still pass when it checks the contents at the end of the test:

        Then the journal should contain "My first entry."
        And the journal should contain "Life is good."

wren · 2021-07-17T21:47:44Z

@minchinweb Yup! We just need to move the tests.

And to clarify what @micahellison is saying, the sample plugin doesn't quite work right now (you can test by installing it locally). So, fixing the test or modifying the test, it should hopefully go into pytest.

minchinweb · 2021-07-17T21:59:05Z

P.S. Note the the Window scenarios in the CI aren't actually running tests...

I get about a dozen failing tests locally (on Windows) and the trackbacks aren't making a lot of sense....

stale · 2021-11-16T12:23:59Z

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.

micahellison · 2021-11-20T19:55:24Z

Closing due to staleness. Feel free to re-open with new contributions.

minchinweb mentioned this pull request Jun 27, 2021

Namespace-based Export Plugins #1006

Closed

wren reviewed Jul 3, 2021

View reviewed changes

jrnl/plugins/exporter/dates.py Outdated Show resolved Hide resolved

wren reviewed Jul 13, 2021

View reviewed changes

wren requested changes Jul 13, 2021

View reviewed changes

wren marked this pull request as draft July 13, 2021 19:55

minchinweb force-pushed the custom-importer branch from 4b72910 to c535ae7 Compare July 14, 2021 03:29

minchinweb marked this pull request as ready for review July 14, 2021 03:38

minchinweb and others added 4 commits July 17, 2021 12:47

Restore __main__.py functionality (jrnl-org#1278)

beb6108

Allows you to access jrnl by running `python -m jrnl`

Fix testing for plugins (jrnl-org#1277)

183795d

* Move plugins tests back into normal test workflow * change when tests run * make tests slightly more readable in PR

wren force-pushed the feature-plugins branch from 34f3a84 to 183795d Compare July 17, 2021 20:11

Sample Importer: Handle data consistently whether from file or stdin

7f5fbca

Importers: handle writing the journal to disk ourselves (rather than delegating to the plugin) Plugins: First pass at testing custom importer

wren force-pushed the custom-importer branch from c535ae7 to 7f5fbca Compare July 17, 2021 20:29

Fix behave test that was being missed due to "Outline" keyword; also …

15786df

…adding in appropriate parameters and no error step

wren force-pushed the feature-plugins branch from 29235a9 to 79c3740 Compare July 17, 2021 21:58

wren marked this pull request as draft September 11, 2021 21:07

stale bot added the stale Inactive issue: will be closed soon if no activity label Nov 16, 2021

micahellison closed this Nov 20, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Custom importer Improvements #1281

Custom importer Improvements #1281

minchinweb commented Jun 27, 2021 •

edited

Loading

micahellison commented Jul 3, 2021

wren Jul 13, 2021

wren Jul 13, 2021

wren Jul 13, 2021

wren left a comment

minchinweb commented Jul 14, 2021

wren commented Jul 17, 2021

minchinweb commented Jul 17, 2021 •

edited

Loading

micahellison commented Jul 17, 2021

wren commented Jul 17, 2021

minchinweb commented Jul 17, 2021 •

edited

Loading

stale bot commented Nov 16, 2021

micahellison commented Nov 20, 2021

Custom importer Improvements #1281

Custom importer Improvements #1281

Conversation

minchinweb commented Jun 27, 2021 • edited Loading

micahellison commented Jul 3, 2021

wren Jul 13, 2021

Choose a reason for hiding this comment

wren Jul 13, 2021

Choose a reason for hiding this comment

wren Jul 13, 2021

Choose a reason for hiding this comment

wren left a comment

Choose a reason for hiding this comment

minchinweb commented Jul 14, 2021

wren commented Jul 17, 2021

minchinweb commented Jul 17, 2021 • edited Loading

micahellison commented Jul 17, 2021

wren commented Jul 17, 2021

minchinweb commented Jul 17, 2021 • edited Loading

stale bot commented Nov 16, 2021

micahellison commented Nov 20, 2021

minchinweb commented Jun 27, 2021 •

edited

Loading

minchinweb commented Jul 17, 2021 •

edited

Loading

minchinweb commented Jul 17, 2021 •

edited

Loading