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

Fix installing from a readonly copy of synapse #866

Merged
merged 2 commits into from
May 14, 2020
Merged

Conversation

richvdh
Copy link
Member

@richvdh richvdh commented May 7, 2020

It seems that, as of pip 20.1, pip likes to do an in-tree build of synapse when
you install it, which is pretty nasty when pip is running in a docker image.

We can work around this by copying the bits we need into a tarball and
installing from that. Ultimately I'd like to change the build pipelines to
build a wheel upfront, and use that for the sytest builds, which will be a more
accurate representation of how most people install synapse, but this is a
prerequisite.

See also: https://discuss.python.org/t/installing-from-a-read-only-source/4115/3

It seems that, as of pip 20.1, pip likes to do an in-tree build of synapse when
you install it, which is pretty nasty when pip is running in a docker image.

We can work around this by copying the bits we need into a tarball and
installing from that. Ultimately I'd like to change the build pipelines to
build a wheel upfront, and use that for the sytest builds, which will be a more
accurate representation of how most people install synapse, but this is a
prerequisite.
@richvdh
Copy link
Member Author

richvdh commented May 7, 2020

(worth noting that this only becomes a problem if/when we rebuild the sytest-synapse docker images)

@richvdh richvdh requested a review from a team May 11, 2020 12:17
Comment on lines 29 to 32
#su -c psql postgres <<< "show config_file"
#su -c psql postgres <<< "show max_connections"
#su -c psql postgres <<< "show full_page_writes"
#su -c psql postgres <<< "show fsync"
Copy link
Member

Choose a reason for hiding this comment

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

Should we remove these if they're not useful?

Copy link
Member Author

Choose a reason for hiding this comment

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

done in 0165ef9

scripts/synapse_sytest.sh Outdated Show resolved Hide resolved
@@ -110,6 +110,24 @@ elif [ -n "$POSTGRES" ]; then

fi

# default value for SYNAPSE_SOURCE
: ${SYNAPSE_SOURCE:=/src}
Copy link
Member

Choose a reason for hiding this comment

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

Neat, did not know this about bash. (https://wiki.bash-hackers.org/syntax/pe#assign_a_default_value if anyone else doesn't know what this does)

Copy link
Member

@clokep clokep left a comment

Choose a reason for hiding this comment

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

Looks plausible. See the minor comments.

@richvdh richvdh merged commit 9fa5157 into develop May 14, 2020
@richvdh richvdh deleted the rav/fix_readonly branch May 14, 2020 16:12
pull bot pushed a commit to valkum/sytest that referenced this pull request May 14, 2020
It seems that, as of pip 20.1, pip likes to do an in-tree build of synapse when
you install it, which is pretty nasty when pip is running in a docker image.

We can work around this by copying the bits we need into a tarball and
installing from that. Ultimately I'd like to change the build pipelines to
build a wheel upfront, and use that for the sytest builds, which will be a more
accurate representation of how most people install synapse, but this is a
prerequisite.
anoadragon453 added a commit that referenced this pull request Jun 10, 2020
* release-v1.14.0:
  Add API environment variable for Dendrite monolith (#877)
  Use /sync not /events
  Use /sync not /events
  Use /sync not /events for test 'Outbound federation can request missing events'
  Review comments
  Fix race in 'Outbound federation can backfill events'
  Deflake guest user test (#871)
  Pass through DENDRITE_TRACE_ env vars to the dendrite binary
  Fix installing from a readonly copy of synapse (#866)
  Remove race condition in dendrite
  Remove debug
  More /sync less /events
  Use /sync not /events to fetch data
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