Skip to content

api: Add bindings for fetching user avatar. - #919

Open
PieterCK wants to merge 1 commit into
zulip:mainfrom
PieterCK:add-binding-for-fetching-user-avatar
Open

api: Add bindings for fetching user avatar.#919
PieterCK wants to merge 1 commit into
zulip:mainfrom
PieterCK:add-binding-for-fetching-user-avatar

Conversation

@PieterCK

@PieterCK PieterCK commented Jun 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Prep PR for: zulip/zulip#32495 (issue: zulip/zulip#32103)

CZO: #api documentation > Python examples that require importing other modules. @ 💬

Self-review checklist
  • Self-reviewed the changes for clarity and maintainability
    (variable names, code reuse, readability, etc.).

Communicate decisions, questions, and potential concerns.

  • Explains differences from previous plans (e.g., issue description).
  • Highlights technical choices and bugs encountered.
  • Calls out remaining decisions and concerns.
  • Automated tests verify logic where appropriate.

Individual commits are ready for review (see commit discipline).

  • Each commit is a coherent idea.
  • Commit message(s) explain reasoning and motivation for changes.

Completed manual review and testing of the following:

  • Visual appearance of the changes.
  • Responsiveness and internationalization.
  • Strings and tooltips.
  • End-to-end functionality of buttons, interactions and flows.
  • Corner cases, error conditions, and easily imagined bugs.

@PieterCK
PieterCK force-pushed the add-binding-for-fetching-user-avatar branch from 2bb6e7c to 14f68ee Compare June 15, 2026 14:47
@zulipbot zulipbot added size: L and removed size: M labels Jun 15, 2026
@PieterCK
PieterCK force-pushed the add-binding-for-fetching-user-avatar branch 6 times, most recently from ab9c2ad to b68598e Compare June 15, 2026 15:45
PieterCK
PieterCK commented Jun 15, 2026
View reviewed changes
Comment thread zulip/zulip/__init__.py Resolved
@PieterCK
PieterCK force-pushed the add-binding-for-fetching-user-avatar branch from b68598e to 18bb73b Compare June 15, 2026 15:51
@PieterCK PieterCK mentioned this pull request Jun 15, 2026
Draft
12 tasks
@PieterCK

PieterCK commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator Author

@prakhar1144 -- Could you please review this one?

@chrisbobbe

chrisbobbe commented Jun 16, 2026

Copy link
Copy Markdown
Member

How is authentication normally handled in the bindings? We should take care not to follow off-realm redirects with auth headers included. I'm not sure whether this revision handles that correctly or not.

@PieterCK

This comment was marked as outdated.

Sign in to view
@chrisbobbe

chrisbobbe commented Jul 20, 2026

Copy link
Copy Markdown
Member

That sounds reasonable (although I haven't done that manual testing myself).

@chrisbobbe

chrisbobbe commented Sep 9, 2026

Copy link
Copy Markdown
Member

@prakhar1144 would you be able to do the next review of this?

prakhar1144
prakhar1144 reviewed Oct 16, 2026
View reviewed changes
Comment thread zulip/zulip/__init__.py Resolved
Comment thread zulip/zulip/__init__.py Resolved
Comment thread zulip/zulip/examples/get-user-avatar Resolved
Comment thread zulip/zulip/examples/get-user-avatar Resolved
Comment thread zulip/zulip/__init__.py Resolved
Comment thread zulip/zulip/__init__.py Resolved
Comment thread zulip/zulip/examples/get-user-avatar Resolved
Comment thread zulip/zulip/__init__.py Resolved
@PieterCK
PieterCK force-pushed the add-binding-for-fetching-user-avatar branch 7 times, most recently from f8acea3 to 5cd143b Compare October 20, 2026 09:51
@PieterCK
PieterCK marked this pull request as draft October 20, 2026 09:52
@PieterCK
PieterCK marked this pull request as ready for review October 20, 2026 11:59
@PieterCK

PieterCK commented Oct 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

@prakhar1144 -- Thanks for reviewing this. I think this is ready for you to take a look!

The failing CI looks like it's caused an outdated dependency in main or something.

@prakhar1144

prakhar1144 commented Oct 22, 2026

Copy link
Copy Markdown
Member

Looks good, apart from this one comment: #919 (comment)

@PieterCK
api: Add bindings for fetching user avatar.
3f2a94b 
This adds methods for fetching user avatars to the Client class. There
is some plumbing in call_endpoint to make sure it can call endpoints
that don't include the API version string (/api/v1), such as the avatar
endpoints.
@PieterCK
PieterCK force-pushed the add-binding-for-fetching-user-avatar branch from 5cd143b to 3f2a94b Compare October 28, 2026 08:40
@zulipbot zulipbot added size: XL and removed size: L labels Oct 28, 2026
@PieterCK

PieterCK commented Oct 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

@prakhar1144 -- I've updated the bindings to return Union[str, Dict[str, Any]], can you look again?

prakhar1144
prakhar1144 approved these changes Oct 28, 2026
View reviewed changes

@prakhar1144 prakhar1144 left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me. Tested it locally.

The CI failure is unrelated, but we should fix it separately.

As the next step, I think someone with permission should do a release with this change. (I'm not sure if we need another round of review from someone on this, we can confirm in CZO)

@PieterCK

PieterCK commented Oct 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

It seems like the CI failure is because Debian 11 (which we use to run the tests on Python 3.9) has reached end of life last August. I opened #934 to drop it from the CI workflow matrix. @prakhar1144 -- would you like to review that PR as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@prakhar1144 prakhar1144 prakhar1144 approved these changes

Assignees

No one assigned

Labels

size: XL

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@PieterCK @chrisbobbe @prakhar1144 @zulipbot