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

Added deploy with google cloud functions #426

Merged
merged 14 commits into from
Jun 20, 2023

Conversation

dat-a-man
Copy link
Collaborator

No description provided.

@netlify
Copy link

netlify bot commented Jun 16, 2023

Deploy Preview for dlt-hub-docs ready!

Name Link
🔨 Latest commit 3c266d6
🔍 Latest deploy log https://app.netlify.com/sites/dlt-hub-docs/deploys/6491b7dbb801cb0008c609a6
😎 Deploy Preview https://deploy-preview-426--dlt-hub-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@dat-a-man dat-a-man requested a review from adrianbr June 16, 2023 12:46
@rudolfix rudolfix self-requested a review June 16, 2023 14:14
Copy link
Collaborator

@rudolfix rudolfix left a comment

Choose a reason for hiding this comment

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

hey @dat-a-man was this cloud function running at all? because it looks like the env variable is not correctly used. but maybe I'm wrong :)

Copy link
Collaborator

@rudolfix rudolfix left a comment

Choose a reason for hiding this comment

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

hey @dat-a-man please fix the PR as specified. I have working direct google secrets in mainpipelinenotion6 but I can add it when other things are fixed


info = pipeline.run(data)
print(info)
return "Data loaded successfully". # returns the value
Copy link
Collaborator

Choose a reason for hiding this comment

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

please return str(info) instead! and yiu see a nice load info

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

return str(info) is not working, its Namerror.

Copy link
Collaborator Author

@dat-a-man dat-a-man Jun 19, 2023

Choose a reason for hiding this comment

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

tried some things earlier too, it would be nice if the function could return load_info

Copy link
Collaborator Author

@dat-a-man dat-a-man Jun 19, 2023

Choose a reason for hiding this comment

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

Any solution to this? Please

Copy link
Collaborator

@rudolfix rudolfix left a comment

Choose a reason for hiding this comment

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

Probably my previous review was not clear:
I think we should give user an option to not use the Secrets Manager at all. That was option 1 I wrote in my previous review. You can add env variable API_KEY with the right value without a need to set up permissions to secret manager. I'd offer this to the user

and what you describe in ## 3. I'd offer as option 2

- Click 'Add a secret reference' and select the secret you created, for example 'notion_secret'.
- Set the 'Reference method' to 'Mounted as environment variable'.
- In the 'Environment Variable' field, enter the name of the environment variable that corresponds to the argument required by the pipeline. - Remember to capitalize the variable name if it is required by the pipeline and specified in secrets.toml. For example, if the variable name is api_key and you have defined the value in the secrets manager secret as "notion_secret", you would declare the environment variable as "API_KEY".
- Finally, click 'DEPLOY' to deploy the function. The HTTP trigger will now successfully execute the pipeline every time the URL is triggered.
Copy link
Collaborator

Choose a reason for hiding this comment

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

here the info on adding permissions to read (Secret Accessor) is actually useful :) sorry for being pedantic. but I'd like to have a good working deployment because we'll implement it in dlt core soon

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added Sir, please see.

Copy link
Collaborator

@rudolfix rudolfix left a comment

Choose a reason for hiding this comment

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

cleaned up text and added reference from deployments document. all good now

@rudolfix rudolfix merged commit d7eb743 into devel Jun 20, 2023
26 checks passed
@rudolfix rudolfix deleted the docs/deploy-google-cloud-functions-pipeline branch June 20, 2023 14:34
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.

None yet

4 participants