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 required fields to POST /keys/upload #1197

Closed
wants to merge 4 commits into from
Closed

Conversation

S7evinK
Copy link
Contributor

@S7evinK S7evinK commented Feb 24, 2022

This adds the required fields to device_keys if they are missing. (the values probably don't make much sense)

@erikjohnston
Copy link
Member

As per matrix-org/synapse#12081, this is failing because you're trying to upload a different key with the same key ID 🙂

@squahtx
Copy link
Contributor

squahtx commented Feb 28, 2022

@S7evinK could you make the tests happy and also add a sign off (https://matrix-org.github.io/synapse/latest/development/contributing_guide.html#sign-off) ?

Also is the "Fix mistake" commit related to key upload?

@squahtx squahtx added the X-Awaiting-Changes A contributed PR which needs changes and re-review before it can be merged label Feb 28, 2022
@S7evinK
Copy link
Contributor Author

S7evinK commented Feb 28, 2022

I'm actually not sure how to fix this within Sytest. :/

Regarding the "Fix mistake": Not exactly related, but fixes an issue I introduced in #1195

@clokep
Copy link
Member

clokep commented Mar 2, 2022

Regarding the "Fix mistake": Not exactly related, but fixes an issue I introduced in #1195

This is likely best in a separate PR then!

I'm going to remove the review flag from Synapse since this:

  • Needs to fix the tests (I'm not 100% sure what @erikjohnston is suggesting, but it sounds like you need to generate unique key IDs, maybe one for each test?)
  • There's a conflict (probably from the path changes in Use /v3 endpoints #1200).

@clokep clokep removed the request for review from a team March 2, 2022 19:35
@richvdh
Copy link
Member

richvdh commented Mar 2, 2022

Regarding the "Fix mistake": Not exactly related, but fixes an issue I introduced in #1195

this is fixed on develop already.

@S7evinK
Copy link
Contributor Author

S7evinK commented Apr 12, 2024

Not going to spend time on Sytest, so closing this.

@S7evinK S7evinK closed this Apr 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
X-Awaiting-Changes A contributed PR which needs changes and re-review before it can be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants