Skip to content

Conversation

jerrykan
Copy link
Contributor

@jerrykan jerrykan commented Feb 4, 2021

A couple of small fixes to improve the user's experience when using roundup-demo.

My git-remote-hg setup is currently broken some I'm unable to push directly to the SourceForge Mercurial repo. So I'm creating a pull request so either I remember to merge this fixes when I get git-remote-hg working, or someone else merges them and pushes them to SourceForge.

Some templates like `jinja2` and `responsive` need certain settings in
their `config.ini` otherwise the pages won't display correctly. By
pulling in these custom settings when running `roundup-demo` we ensure
that the pages will display correctly out of the box with no extra
changes required.
The `roundup-demo` command assumes that the `user` has a `realname`
attribute when it creates the demo user. Without adding the `realname`
attribute in the `minimal` template's schema an error will occur when a
user tries to create a new demo based on it.
@codecov-io
Copy link

codecov-io commented Feb 4, 2021

Codecov Report

Merging #2 (8900803) into master (d217654) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master       #2      +/-   ##
==========================================
- Coverage   70.14%   70.13%   -0.01%     
==========================================
  Files          96       96              
  Lines       20744    20745       +1     
==========================================
  Hits        14550    14550              
- Misses       6194     6195       +1     
Impacted Files Coverage Δ
roundup/demo.py 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d217654...8900803. Read the comment docs.

@rouilj
Copy link
Member

rouilj commented Feb 4, 2021 via email

@rouilj
Copy link
Member

rouilj commented Feb 17, 2021

Also John, I don't think you need:

Add realname attribute to user in minimal template schema

in demo.py is this bit of code:

    # FIXME: Move tracker-specific demo initialization into the tracker
    # templates.
    if template == 'minimal':
        db.user.create(username='demo', password=password.Password('demo'),
                       roles='User')
    else:
        db.user.create(username='demo', password=password.Password('demo'),
                       realname='Demo User', roles='User')

whish should not try initializing the realname property if using the minimal template. Maybe the bug that needs
to be fixed is that template == 'minimal' is not true for the minimal template???

@jerrykan
Copy link
Contributor Author

jerrykan commented Feb 26, 2021

whish should not try initializing the realname property if using the minimal template. Maybe the bug that needs
to be fixed is that template == 'minimal' is not true for the minimal template???

It seems that def install_demo(home, backend, template) may have two different types of string value depending on where it is invoked from.

When invoked from roundup/scripts/roundup_demo.py (ie. by running roundup-demo) the install_demo() function is passed a template value that is the full path to the template directory. In this case the template == 'minimal' won't match and will result in an Exception.

When invoked from roundup/demo.py (ie. by running demo.py) the install_demo() function is passed a template value that is just the name of the template. In this case the template == 'minimal' will match the template == 'minimal' condition and work as expected.

I should also note that install_demo() makes some assumptions about he location of the templates, so running demo.py doesn't work unless your working directory is the same as where the demo.py script is located. On the other hand round-admin works correctly when run from any working directory.

@rouilj
Copy link
Member

rouilj commented Mar 2, 2021

Ah thanks for tracking that down. I didn't even realize there was a roundup-demo to tell the truth. I wonder what the use
case for that is. I guess is allows a roundup demo for roundup installed via an OS package?

What do you think about fixing template == 'minimal' check so that it strips the path from template (os.path.basename(template)) and only compares the last component. That should fix the roundup-demo
case when used with the installed version of the distributed minimal template.

I am ok with requiring the current working directory be the same directory where demo.py is located. IIRC all of our
docs have that assumption as well.

Thoughts?

@jerrykan
Copy link
Contributor Author

jerrykan commented Mar 9, 2021

Ah thanks for tracking that down. I didn't even realize there was a roundup-demo to tell the truth. I wonder what the use
case for that is. I guess is allows a roundup demo for roundup installed via an OS package?

Using roundup-demo is the easiest (only?) way to init a demo instance after doing a pip install roundup. Given that pip is the standard way to install python packages maybe we should be optimising for that use-case, instead of requiring user to checkout the source code to run demo.py?

@jerrykan
Copy link
Contributor Author

jerrykan commented Mar 9, 2021

Maybe we should move the discussion about the demo stuff to the mailing list? I didn't mean to subvert other communication channels by opening a "placeholder" pull request :D

@rouilj
Copy link
Member

rouilj commented Mar 10, 2021 via email

@jerrykan
Copy link
Contributor Author

Discussion moved to the mailing list: https://sourceforge.net/p/roundup/mailman/message/37240835/

@jerrykan
Copy link
Contributor Author

jerrykan commented Apr 12, 2021

Just adding a note here for when work on these patches get picked-up and worked on again. It might be work looking at using the setuptools ResourceManager API as a more portable/reliable way to accessing template files.

@rouilj
Copy link
Member

rouilj commented Dec 9, 2021

Committed in http://sourceforge.net/p/roundup/code/ci/5a3a386aa8e7

Regarding ResourceTools, I guess I am too dumb to see how that helps deal with the man page, template dir, locale file resources.

@rouilj rouilj closed this Dec 9, 2021
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.

3 participants