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

Remove type=global from /spaces api call #92

Merged

Conversation

mikecsmith
Copy link
Contributor

Context

Passing the type=global query param in the spaces API call prevents md2conf from being used with knowledge_base spaces.

Note

I did consider allowing the user to pass in the type via a --space-type flag with a default to global but my Python isn't fantastic. That feels like it would be a significant improvement over this PR but may not be necessary.

Contributing

I've read CONTRIBUTING.md and ran check.sh prior to raising the PR.

Forcing type=global in the /spaces API call prevents md2conf from being
used with knowledge_base spaces
@@ -194,7 +194,7 @@ def space_id_to_key(self, id: str) -> str:
payload = self._invoke(
ConfluenceVersion.VERSION_2,
"/spaces",
{"ids": id, "type": "global", "status": "current"},
{"ids": id, "status": "current"},
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at Confluence REST API v2 documentation for spaces, type doesn't appear to be a required parameter. Originally, functions space_id_to_key and space_key_to_id were eager, and they would fetch all spaces in Confluence, which is why I added the filter to global to ensure we have fewer results. However, I have recently switched to lazy evaluation, with the space key traded to space ID (or vice versa) when necessary, e.g. a page is about to be created, and the space key in the Markdown file has to be exchanged to a space ID. This means that the query response has at most a single result, and we no longer need the filter to global.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the context and much appreciated on the fast review/approval. I wasn't sure if the type: "global" had a specific use case but this helps to clear that up.

Brilliant project and thanks for open sourcing it!

@hunyadi hunyadi merged commit 3f14ba5 into hunyadi:master Mar 5, 2025
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.

2 participants