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

Add cloud provider prefix to deployment ID #1480

Merged
merged 2 commits into from
Sep 30, 2022

Conversation

amarthadan
Copy link
Contributor

Close #1473

@amarthadan amarthadan requested a review from a team September 28, 2022 09:59
@amarthadan amarthadan self-assigned this Sep 28, 2022
@amarthadan amarthadan linked an issue Sep 28, 2022 that may be closed by this pull request
@@ -511,6 +512,79 @@ export type Deployment = {
versions?: DeploymentVersion[];
};

// If deploymentId is provided tries fetch only that one deployment and finishes early when found
Copy link
Contributor

Choose a reason for hiding this comment

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

If deploymentId is provided it tries to fetch only that one deployment and returns early when found

@@ -511,6 +512,79 @@ export type Deployment = {
versions?: DeploymentVersion[];
};

// If deploymentId is provided tries fetch only that one deployment and finishes early when found
async function fetchDeployments(cloudProvider: CloudProvider['type'], deploymentId?: string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I think we will be using type CloudProvider['type'] so consider defining it as type CloudProviderType = CloudProvider['type']

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm currently replacing it with a full CloudProvider type at some places so I'll see whether it makes sense.


const latestDeployment = Object.keys(stageDirectory.children).sort().reverse()[0];
const bucketConfigPath = `${airnodeAddress}/${stage}/${latestDeployment}/config.json`;
const config = JSON.parse(
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd wrap this in go since this can fail in many ways (users manually edit aws config, we change the region and nodeVersion field). It would be nice to handle this gracefully.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll do that as part of #1449

const { id, cloudProvider, region, airnodeAddress, stage, airnodeVersion, lastUpdate, versions } =
deployment as Deployment;
const deployment = goCloudDeploymentInfo.data[0];
const { id, region, airnodeAddress, stage, airnodeVersion, lastUpdate, versions } = deployment as Deployment;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this not typed automatically? The go utils should provide inference for these things.

@amarthadan amarthadan requested a review from Siegrift September 29, 2022 09:14
Copy link
Contributor

@Siegrift Siegrift left a comment

Choose a reason for hiding this comment

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

👍 LGTM.

Base automatically changed from deployment-info to temp-v0.10.0 September 30, 2022 11:27
@amarthadan amarthadan merged commit d1f5886 into temp-v0.10.0 Sep 30, 2022
@amarthadan amarthadan deleted the deployment-id-cloud branch September 30, 2022 11:31
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.

Prefix deployment ID with the cloud provider identification
2 participants