Skip to content

Conversation

@sfc-gh-echristensen
Copy link

Description

This is an example application for creating a pg_lake application with PostGIS calling Overture maps data.

This update does not change the core functionality of pg_lake in any way. Just adds some example code that a user may way to build from or test with.

@sfc-gh-abozkurt
Copy link
Collaborator

That is nice, but I think we should not merge that without api tests automated by CI pipeline. Otherwise, it would be manual maintenance burden all the time.

Copy link
Collaborator

@sfc-gh-okalaci sfc-gh-okalaci left a comment

Choose a reason for hiding this comment

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

hm, that's way too large than I expected. Tbh, I was expecting to see few sql files, and that's it.

Now, we are trying to merge an application to pg_lake, where maintaining the correctness of this app is going to cause a lot of pain for us.

I think it is fair that we cannot merge the PR like this. If we really want to merge, we should have a proper CI pipeline that runs this and make sure it is not broken.

To me, that's a lot of effort with little benefit on pg_lake repo. Do you mind if we don't merge this PR at all? Otherwise, we might require you to add the proper CI pipeline, and automated tests for this piece.

(and, I know I mentioned that it is fine to add to pg_lake in an internal chat, but as noted, I was not expecting something like this, sorry for the misguidance)

@sfc-gh-abozkurt
Copy link
Collaborator

That is nice, but I think we should not merge that without api tests automated by CI pipeline. Otherwise, it would be manual maintenance burden all the time.

One idea is that we can create a separate repo which can include example cases of pg_lake, which can have its own tests and CI. It might add pg_lake as submodule. It sounds better to me that we can refer this example repo from pg_lake's README.md.

@sfc-gh-echristensen
Copy link
Author

@sfc-gh-okalaci @sfc-gh-abozkurt Ok - if you think this is too big or complicated for the pg_lake repo I'll just create a separate one for this and folks can just find it on their own.

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