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

feat: add kv binding in actions for referral tracker #163

Open
wants to merge 13 commits into
base: development
Choose a base branch
from

Conversation

zugdev
Copy link
Collaborator

@zugdev zugdev commented Nov 8, 2024

Resolves #151

@gentlementlegen I know this ain't right as of now, but I'd love some feedback. I've based myself of https://github.com/ubiquity-os/ubiquity-os-kernel/.

@zugdev zugdev requested a review from 0x4007 as a code owner November 8, 2024 03:23
@ubiquity-os-deployer
Copy link

ubiquity-os-deployer bot commented Nov 8, 2024

@gentlementlegen
Copy link
Member

gentlementlegen commented Nov 8, 2024

@zugdev that's the spirit, but removing ubiquity/cloudflare-deploy-action@main in favor of cloudflare/wrangler-action@v3 might have side effects. I don't know all the work that happens behind the scenes but I think the first Action takes care of previews and building URLs and such. @0x4007 might know more. You'd want to take a look at the source code : https://github.com/ubiquity/cloudflare-deploy-action/. It is maybe even better to actually update that package directly.

@0x4007
Copy link
Member

0x4007 commented Nov 18, 2024

I don't know more.

@zugdev
Copy link
Collaborator Author

zugdev commented Nov 18, 2024

I'll test it out.

@zugdev
Copy link
Collaborator Author

zugdev commented Nov 29, 2024

@0x4007 can you pls give me actions permission for me to try running this deploy workflow file?

@0x4007
Copy link
Member

0x4007 commented Dec 12, 2024

@0x4007 can you pls give me actions permission for me to try running this deploy workflow file?

I missed this. Are you asking to be admin? I'll do that now

[env.dev]
[[env.dev.kv_namespaces]]
binding = "kv"
id = "9cd5fdf01971402390dbe9e95cc078d3"
Copy link
Member

Choose a reason for hiding this comment

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

This is what we are trying to resolve, it should not be here, the deployment will fail.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We gotta have the production fields else update toml fails cause it doesnt find a field to change

Copy link
Member

Choose a reason for hiding this comment

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

The kernel does not use any of these, they actually should get removed.

Yes but I think the fields can be empty, like for plugins. Example: https://github.com/ubiquity-os-marketplace/command-start-stop/blob/development/wrangler.toml

@zugdev
Copy link
Collaborator Author

zugdev commented Jan 24, 2025

Managed to make it work by copying kernel, here is QA:

https://github.com/ubiquity/work.ubq.fi/actions/runs/12944850895

@gentlementlegen can you take a quick look?

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.

Add variables into wrangler deployment script
3 participants