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

Create geography_type enum and add geography column in connection table #16818

Merged
merged 12 commits into from
Oct 5, 2022

Conversation

terencecho
Copy link
Contributor

@terencecho terencecho commented Sep 16, 2022

What

In preparation to support a routing service for jobs to be put in different task queues based off the data planes users want to run them in, this PR creates a geography enum type in the database (currently populated with AUTO, US and EU) and creates a geography column in the connection table.

@terencecho terencecho changed the title Tcho/geography column Create geography_type enum and add geography column in connection table Sep 16, 2022
@terencecho terencecho temporarily deployed to more-secrets September 16, 2022 18:36 Inactive
@terencecho terencecho temporarily deployed to more-secrets September 16, 2022 21:37 Inactive
@terencecho terencecho temporarily deployed to more-secrets September 19, 2022 00:42 Inactive
@terencecho terencecho temporarily deployed to more-secrets September 20, 2022 16:27 Inactive
@terencecho terencecho temporarily deployed to more-secrets September 20, 2022 17:34 Inactive
@terencecho terencecho marked this pull request as ready for review September 20, 2022 17:40
@terencecho terencecho requested a review from pmossman September 20, 2022 17:41
@terencecho terencecho temporarily deployed to more-secrets September 21, 2022 15:00 Inactive
@terencecho terencecho temporarily deployed to more-secrets September 21, 2022 15:37 Inactive
@terencecho terencecho temporarily deployed to more-secrets September 22, 2022 14:36 Inactive
@terencecho terencecho temporarily deployed to more-secrets September 22, 2022 18:39 Inactive
@terencecho terencecho temporarily deployed to more-secrets September 29, 2022 22:18 Inactive
@terencecho
Copy link
Contributor Author

todo: edit name of file to reflect the current oss version

Copy link
Contributor

@pmossman pmossman left a comment

Choose a reason for hiding this comment

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

Sorry I took a while to get to this, looks good!

.execute();
}

private static void addGeographyColumn(final DSLContext ctx) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@terencecho just realized, I think we want all existing connection record to be set to 'AUTO' during this migration, right?

@pmossman
Copy link
Contributor

pmossman commented Oct 3, 2022

@terencecho I started on the API changes that will use this, and I realized we might want to make some changes to this migration before we merge/run it in prod.

  • I think it probably makes sense to just add the column to the workspace table as well as connections in a single migration, I think it isn't much of a lift and we might as well just knock out both changes in one migration since they're conceptually related
  • I think we should make the column explicitly non-nullable, and declare a default value of 'auto' in the migration so that existing workspaces and connection records get set to 'auto' during the migration

Let me know if any of that doesn't make sense or if you wanna chat about it!

@terencecho
Copy link
Contributor Author

@pmossman sounds good to me. i can make those changes on tuesday.

@terencecho terencecho temporarily deployed to more-secrets October 4, 2022 23:10 Inactive
@terencecho terencecho temporarily deployed to more-secrets October 5, 2022 18:36 Inactive
@terencecho terencecho temporarily deployed to more-secrets October 5, 2022 19:16 Inactive
@pmossman pmossman temporarily deployed to more-secrets October 5, 2022 20:28 Inactive
@pmossman
Copy link
Contributor

pmossman commented Oct 5, 2022

New updates since my last review look good, consider this re-approved!

@pmossman pmossman temporarily deployed to more-secrets October 5, 2022 20:58 Inactive
@terencecho terencecho merged commit a7a1c7f into master Oct 5, 2022
@terencecho terencecho deleted the tcho/geography-column branch October 5, 2022 21:39
letiescanciano added a commit that referenced this pull request Oct 6, 2022
…vation

* master: (26 commits)
  supply a source id for schema discovery in connector integration tests (#17662)
  Source Iterable: Add permission check for stream (#17602)
  Moving TrackingClientSingleton.initialize into the bean itself (#17631)
  Handle null workspace IDs in tracking/reporting methods gracefully (#17641)
  Bump Airbyte version from 0.40.11 to 0.40.12 (#17653)
  Revert "Do not wait the end of a reset to return an update (#17591)" (#17640)
  Standardize HttpRequester's url_base and path format (#17524)
  Create geography_type enum and add geography column in connection and workspace table (#16818)
  airbyte-cron: update connector definitions from remote (#16438)
  Do not wait the end of a reset to return an update (#17591)
  Remove redundant title labels from connector specs (#17544)
  Updated GA4 status
  support large schema discovery (#17394)
  🪟 🐛 Fixes connector checks not properly ending their loading state (#17620)
  🪟🧪 [Experiment] add hideOnboarding experiment (#17605)
  Source Recharge: change releaseStage to GA (#17606)
  Source Recharge: skip stream if 403 received (#17608)
  remove sonar-scan workflow (#17609)
  Mark/tables should be full width on all pages (#17401)
  Auto fail all workfow if there is a Versioning issue (#17562)
  ...
jhammarstedt pushed a commit to jhammarstedt/airbyte that referenced this pull request Oct 31, 2022
… workspace table (airbytehq#16818)

* init commit of geography_type and column

* add schema dump

* confirm prod database was meant to be changed

* add AUTO to geography

* set default and not nullable, also edit workspace table

* update version number

* update schema dump

Co-authored-by: pmossman <parker@airbyte.io>
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