confluencebuilder icon indicating copy to clipboard operation
confluencebuilder copied to clipboard

confluence_publish_allowlist unexpected behavior

Open wentz89 opened this issue 3 years ago • 3 comments

This Parameter can either be:

None: Publish all Documents List[str]: only publish these Documents str: Filename with a list of documents to publish

I have a function that checks which RST-Files have changed since the last commit and writes this values into the confluence_publish_allowlist Variable. Naturally if no files have changed i would return an empty list. This will cause an upload of all Documents. Which is regarding the description in the Documentation not the expected behavior.

So i assume that in the Code it check the value of publish_allowlist like this

if not confluence_publish_allowlist: ...

But it should actually check if confluence_publish_allowlist is None: ...

Edit: Probably here builder.py

wentz89 avatar Aug 24 '22 09:08 wentz89

Going to iterate this to ensure I understand correctly, the desire here is to have a provided allow-list (either provided by a list type or a file entry) where if the resulting allow-list is empty, we should not publish any pages?

I would agree that this is not the case right now, did not consider the case of users provided an empty allow-list. It seems to be a valid use case to lean towards restricting any publication with a configured list (when empty) over assuming a list is not provided. I would assume there maybe should be an exception from the command line, where a -Dconfluence_publish_allowlist= is provided, which would maybe be best served as an "unset" request where it defaults to publishing all documents.

jdknight avatar Aug 24 '22 22:08 jdknight

Going to iterate this to ensure I understand correctly, the desire here is to have a provided allow-list (either provided by a list type or a file entry) where if the resulting allow-list is empty, we should not publish any pages?

That's what i meant. I was expecting this behavior from what I've read in the Documentation. I mean one could simply update the documentation to clarify the behavior. On the other hand (at least for me) providing an empty list to not upload any files is handy (doing it in Jenkins Pipeline). Currently I work around and if there are no changes simply pass all files to deny_list....

Looking thru the Code to Patch this may be fairly simple. If needed i could prepare a PR.

I would agree that this is not the case right now, did not consider the case of users provided an empty allow-list. It seems to be a valid use case to lean towards restricting any publication with a configured list (when empty) over assuming a list is not provided. I would assume there maybe should be an exception from the command line, where a -Dconfluence_publish_allowlist= is provided, which would maybe be best served as an "unset" request where it defaults to publishing all documents.

The command line case might be a bit tricky I guess. I'm doing it from the conf.py. Do you have an idea for that? Edit: Reading the second part of your comment again I think understand now what you mean for the command line option: -Dconfluence_publish_allowlist= evaluates to None which means build all files or the other way sphinx-build [options] -D confluence_publish_allowlist=index,foo/bar ... What means you can not pass an empty list via command line (I've tried it passing an empty list and in a small test script and I think its not possible for argparse), but if one likes you can set the parameter to [] in conf.py. Sounds fair to me.

By the way, I also had problems passing a filename via command line and it did not work out. Guess that is not supported as well?

wentz89 avatar Aug 25 '22 09:08 wentz89

For sure there are limitations with the management of this option (probably not even just these options).

-Dconfluence_publish_allowlist= evaluates to None which means build all files

Yup, I'm assuming this is best. I assume anyone who may override the confluence_publish_allowlist option from the command line would be more so interested in unconfiguring the option, instead of hinting "I have an empty list/value".

What means you can not pass an empty list via command line

We could crudely support this by maybe handling -Dconfluence_publish_allowlist=, as an "empty list" (splits into two empty strings, which we can cleanup internally to be an empty list). But... I don't know if there is any value added here for users -- however, maybe you might know more about this, I don't think I have actively used this feature that much.

[Had] problems passing a filename via command line and it did not work out. Guess that is not supported as well?

Correct. It looks like this is not supported.

We possibly could add support for this. If we detect a string that maps to a file name that exists, it is most likely that the user is trying to pass a file entry for this option (since documents lists won't have extension names).

[This] may be fairly simple. If needed i could prepare a PR.

Changes to correct/improve the subset options are welcome and appreciated, if you wanted to prepare a pull request. Most likely just a couple of changes inside sphinxcontrib/confluencebuilder/builder.py, and possibly some tweaks to sphinxcontrib/confluencebuilder/config/checks.py (if needed). And any added unit tests to help expand the sanity checks on this option would be a nice plus.

jdknight avatar Aug 25 '22 22:08 jdknight

Changes (#741) have been made to help restrict publishing with empty allow list (confluence_publish_allowlist). This change has been added into the main branch and should be made available next stable release. If users wish to use this capability before a release is made, the development version of this extension can be installed.

jdknight avatar Jan 02 '23 01:01 jdknight

v2.0 is now available on PyPI -- marking as closed.

jdknight avatar Jan 03 '23 01:01 jdknight