Skip to content

feat: resolve project-id and org-id using instance-id#38

Draft
LucDeCaf wants to merge 3 commits into
mainfrom
feat/optional-id-params
Draft

feat: resolve project-id and org-id using instance-id#38
LucDeCaf wants to merge 3 commits into
mainfrom
feat/optional-id-params

Conversation

@LucDeCaf
Copy link
Copy Markdown

Replaces #37 using a cleaner and (hopefully) simpler approach.


Using a soon-to-be-introduced path in @powersync/management-client, automatically fetch the project_id and org_id flags when given the instance_id for a number of commands (where it makes sense to do so).

Example

Before:

powersync pull instance --project-id='project-123-456-789' --instance-id='instance-123-456-789'

After:

# Project ID is fetched from the management service if required
powersync pull instance --instance-id='instance-123-456-789'

NB: Test failures are expected until the next version of the @powersync/management-client releases.

Comment thread cli/src/api/cloud/validate-cloud-link-config.ts Outdated
Comment thread packages/cli-core/src/utils/resolve-cloud-instance-link.ts Outdated
return { instanceConfig, linked };
}

if (!orgId || !projectId) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The logic here is becoming a bit more difficult to follow. Would it be possible to (pre)populate the orgId and projectId if only the instanceId is provided? Then, I would assume the logic of this function could remain as it was.

Otherwise, maybe lets add some code comments for the various paths.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The reason for the differing paths is because this function is sometimes called with an existing instance backing it and sometimes called when creating a new instance. I split the function into a separate one for each path, so that should make things clearer.

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