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

Add wallabag #235

Closed
wants to merge 4 commits into from
Closed

Add wallabag #235

wants to merge 4 commits into from

Conversation

freaktechnik
Copy link
Contributor

To ease and accelerate the PR submission, please use the template below to provide all requested information.
You can find guidelines on which docker image to use in the Rockstor's documentation,
as well as examples of proper formatting in other rock-ons (here
and here)

General information on project

This pull request proposes to add a new rock-on for the following project:

Information on docker image

Checklist

  • Passes JSONlint validation
  • Entry added to root.json in alphabetical order (for new rock-on only)
  • "description" object lists and links to the docker image used
  • "description" object provides information on the image's particularities (advantage over another existing rock-on for the same project, for instance)
  • "website" object links to project's main website

@phillxnet phillxnet added the needs review Test install, function, on / off behaviour, all links / info. label Aug 16, 2020
@FroggyFlox
Copy link
Member

Hi @freaktechnik ,

First of all, please accept my apologies for not getting to your pull request in so long.

I just tried to have it work a few days ago but can't seem to find a way to properly set the DOMAIN_NAME environment variable. Would you have a working set of options that could be used to test it, or more information?
I'm also wary of having many ENV presented to the user like that if it is not necessary... it's always a hard-to-find balance between offering good customization, and not complicating the setup too much. Would some of these options be configurable in Wallabag's UI, by any chance? I wanted to check that up, but as I couldn't figure out how to configure the DOMAIN_NAME, unfortunately.

Let us know if you any more information, and sorry again for coming with more question after such a long time.

@freaktechnik
Copy link
Contributor Author

freaktechnik commented Oct 29, 2020

DOMAIN_NAME should be the host that wallabag is available from, for example https://example.com. Sadly none of these are configurable within wallabag's UI and are only available from symfony config, so either a yaml file in the project (not very user friendly and hard to do with docker, especially when you can only mount folders) or env vars.

@FroggyFlox
Copy link
Member

Thanks a lot for the prompt answer, I'll try again next time I have some free time for it.

@FroggyFlox
Copy link
Member

DOMAIN_NAME should be the host that wallabag is available from, for example https://example.com.

Hi @freaktechnik , I unfortunately can't seem to get this DOMAIN_NAME right, so would you have a working example? The webpage cannot load assets as it tries to load them from <DOMAIN_NAME>/wallassets/ but that just doesn't seem to work in my case. Could that be because one needs a proper domain name for this to work?

Sorry for asking the same thing yet again, I fear this is something obvious I'm missing...

@freaktechnik
Copy link
Contributor Author

I would assume so. I have mine deployed at later.humanoids.be which uses exactly this definition.

@FroggyFlox
Copy link
Member

I see, thanks for the confirmation. We might want to detail that in the rock-on description with something along the lines of: "this rock-on requires a dedicated domain name for proper function" to limit the chances of users having a bad surprise.

On a side note, I wonder if one can get around that requirement by having a reverse proxy properly setup locally...

@freaktechnik
Copy link
Contributor Author

Based on

Full URL of your wallabag instance (without the trailing slash)

(https://doc.wallabag.org/en/admin/parameters.html)
I'd expect that any host name with protocol should work, though I haven't tried using a static IP, for example.

@FroggyFlox
Copy link
Member

I'd expect that any host name with protocol should work, though I haven't tried using a static IP, for example.

From my tests, the page loads, but all CSS fails to load, for instance, as it seems to be trying to fetch all assets from <DOMAIN_NAME>/wallassets/, thereby requiring the DOMAIN_NAME to be a valid setting.

After a quick look around online, it appears some people go around that by using a separate container providing a web server/reverse proxy... that could be an interesting solution to the problem, but not for this PR. I believe we can simply make it clear to the user in the description that a proper domain name must be already setup for this rock-on to work as intended so that we limit bad user experiences. I'll try to get to making proper suggestions to the PR when I get some time and test them.

Thanks again, and sorry for the repeated hassle... Wallabag is a very good addition, but it's too bad it's not the most user-friendly to setup.

@freaktechnik
Copy link
Contributor Author

It is the most user friendly to setup of all the rockons I've created so far, thus this PR :)

@phillxnet
Copy link
Member

phillxnet commented Nov 24, 2021

@freaktechnik if you are still interested in this Rock-on. Do you fancy re-basing on latest master as I've unfortunately broken your pr via some recent removals of deprecated rock-ons.
If it's now a lost cause / of no interest then please accept my apologies for the long tail and close the pr as-is.

@freaktechnik
Copy link
Contributor Author

There you are

@phillxnet
Copy link
Member

Noting @FroggyFlox's outstanding short-fall on this one:

We might want to detail that in the rock-on description with something along the lines of: "this rock-on requires a dedicated domain name for proper function" to limit the chances of users having a bad surprise.

As an aid in what is holding up final testing and hopefully a merge.

@freaktechnik Thanks for hanging in there on this one, and for doing the re-base. Much appreciated.
Hopefully in time we can 'expand' the Rock-on capabilities to avoid these corner cases of having to do post install config.

@phillxnet phillxnet added the rebase-request Please rebase on current master label Sep 27, 2022
@phillxnet
Copy link
Member

@freaktechnik My apologies but as part of some major repo clean up (deleting broken Rock-ons) we now have a need for this pr to be rebased again. Let us know if you are still interested. We are in the throws of another merge session within this repo it seems.

@freaktechnik
Copy link
Contributor Author

There you are.

@phillxnet
Copy link
Member

@freaktechnik Just been doing some repo tidy-up, Thanks for seeing to the past re-base here.
Image updated 2 days ago so still alive.
Also both amd64 and arm64 compatible so that's good.

@FroggyFlox You have already had a little look at this Rock-on. Bar the missing arch compatibility within description, are you able to test this. I don't have a spare domain myself. Your request re this requirement is now in however:

wallabag-rock-on-listing

@phillxnet
Copy link
Member

Closing as per this wave of repo house cleaning. Do please re-open upon continued interest. This does look like a nice Rock-on to have. But testing is tricky as has been noted by @FroggyFlox .

Thanks to @freaktechnik and @FroggyFlox for the work done to date. We just struggle with review resources (human) so I'm doing a repo tidy to help focus the repo on requests with current interest.

@phillxnet phillxnet closed this Feb 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs review Test install, function, on / off behaviour, all links / info. rebase-request Please rebase on current master
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants