Skip to content

[FeedlyUser] factorize lazy streams and add simple examples#20

Merged
Mathieu4141 merged 11 commits intomasterfrom
feedly-user/factorize-lazy-streams
Dec 15, 2021
Merged

[FeedlyUser] factorize lazy streams and add simple examples#20
Mathieu4141 merged 11 commits intomasterfrom
feedly-user/factorize-lazy-streams

Conversation

@Mathieu4141
Copy link
Member

  • Factorizing the behaviour to lazily get streams in the FeedlyUser class. I wanted to add support to get the stream by names but with the previous caching system it needed to be done in 4 places
  • Adding 2 simple examples:
    • Streaming entries
    • Listing categories and tags

@@ -9,19 +9,19 @@
import sys
Copy link
Member Author

Choose a reason for hiding this comment

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

forgot to commit the black reformatting in previous PR

data: Dict = None,
timeout: int = None,
max_tries: int = None,
) -> Response:
Copy link
Member Author

Choose a reason for hiding this comment

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

I need to access the response headers to get the continuation in the IoC use case

raise ValueError(f"Stream `{name}` not found. Available streams: {list(self.name2stream)}") from None

def get_from_id(self, id: str) -> StreamableT:
if UUID_REGEX.match(id):
Copy link
Contributor

Choose a reason for hiding this comment

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

alternatively, you could try to parse the string as UUID: if it fails, it's not a valid uuid

Comment on lines +52 to +53
client_id: str = "feedlydev",
client_secret: str = "feedlydev",
Copy link
Contributor

Choose a reason for hiding this comment

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

doesn't seem used for now

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

sorry, I thought you added these lines
Interesting, something to consider in a follow up PR

@Mathieu4141 Mathieu4141 merged commit 86cde1c into master Dec 15, 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.

2 participants