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

Use trigger to generate apub URL in insert instead of update, and fix query planner options not being set when TLS is disabled #4797

Merged
merged 29 commits into from
Jul 2, 2024

Conversation

dullbananas
Copy link
Collaborator

@dullbananas dullbananas commented Jun 10, 2024

No description provided.

@dullbananas dullbananas reopened this Jun 19, 2024
@dullbananas dullbananas changed the title Replace generate_local_apub_endpoint with triggers Use trigger to generate apub URL in insert instead of update Jun 19, 2024
@dullbananas dullbananas marked this pull request as ready for review June 25, 2024 20:34
// provide a setup function which handles creating the connection
let mut config = ManagerConfig::default();
config.custom_setup = Box::new(establish_connection);
let manager = AsyncDieselConnectionManager::<AsyncPgConnection>::new_with_config(&db_url, config);
Copy link
Member

@Nutomic Nutomic Jun 26, 2024

Choose a reason for hiding this comment

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

This is unrelated, better to put it into a separate PR. Or is it just leftover from merging?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry, forgot to explain this.

This fixes a mistake that caused the configuration updates in the establish_connection function to only run if TLS is enabled, which started to break tests in this PR.

The other change here is adding lemmy.protocol_and_hostname to the config options. batch_execute is no longer used because it doesn't allow bind parameters.

CREATE TRIGGER change_values
BEFORE INSERT ON private_message
FOR EACH ROW
EXECUTE FUNCTION r.private_message_change_values ();
Copy link
Member

Choose a reason for hiding this comment

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

Where is the trigger for comment?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For comments, an existing trigger was updated. It's the first change in this file.

@dullbananas dullbananas changed the title Use trigger to generate apub URL in insert instead of update Use trigger to generate apub URL in insert instead of update, and fix query planner options not being set when TLS is disabled Jun 26, 2024
Copy link
Member

@dessalines dessalines left a comment

Choose a reason for hiding this comment

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

👍

@dessalines dessalines merged commit 78702b5 into LemmyNet:main Jul 2, 2024
1 check passed
Nutomic pushed a commit to sunaurus/lemmy that referenced this pull request Sep 20, 2024
… query planner options not being set when TLS is disabled (LemmyNet#4797)

* Update create.rs

* Update utils.rs

* Update utils.sql

* Update triggers.sql

* Update utils.sql

* Update create.rs

* Update create.rs

* Update create.rs

* Update create.rs

* Update create.rs

* Update create.rs

* Update create.rs

* Update create.rs

* Create up.sql

* Update up.sql

* Update triggers.sql

* Update utils.rs

* stuff

* stuff

* revert some changed files

* Revert "revert some changed files"

This reverts commit 028eabb.

* revert the correct files

* partial reverts

* migration, tests, fix establish_connection

* lint

* pg_format
Nutomic added a commit that referenced this pull request Oct 16, 2024
… and fix query planner options not being set when TLS is disabled (#4797)"

This reverts commit 78702b5.
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.

3 participants