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.

Sign up for GitHub

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

Jump to bottom

Add http headers support when downloading features archives #715

Merged
merged 2 commits into from
Oct 19, 2023
Merged

Add http headers support when downloading features archives #715

merged 2 commits into from
Oct 19, 2023

Conversation

aacebedo
Copy link
Contributor

This feature enables to use devpod with servers needing an authentication.

@aacebedo aacebedo mentioned this pull request Sep 30, 2023
Closed
@aacebedo aacebedo force-pushed the aacebedo/add_http_headers branch 2 times, most recently from 7cede3f to 3d7c051 Compare October 4, 2023 06:41
@aacebedo
Copy link
Contributor Author

aacebedo commented Oct 4, 2023

@89luca89 rebased and fixed the conflicts

@aacebedo
Copy link
Contributor Author

Ping?

pascalbreuninger
pascalbreuninger requested changes Oct 12, 2023
View reviewed changes
Copy link
Member

@pascalbreuninger pascalbreuninger left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution!
Added two comments we should address before merging :)

Additionally, would be great if you could add a test for this in e2e/up

pkg/devcontainer/feature/features.go Show resolved Hide resolved
docs/pages/developing-in-workspaces/devcontainer-json.mdx Outdated Show resolved Hide resolved
@aacebedo
Copy link
Contributor Author

aacebedo commented Oct 14, 2023 •
edited
Loading

Thanks for your contribution! Added two comments we should address before merging :)

Additionally, would be great if you could add a test for this in e2e/up

For the e2e/up test I think I'll need a zip package containing a dummy feature and stores it on a private package registry of the devpod repo.
Would it be ok for you ? And are you able to store this package in a private registry?

Added a test and simulates the HTTP server during execution.

@pascalbreuninger
Copy link
Member

Thanks for your contribution! Added two comments we should address before merging :)
Additionally, would be great if you could add a test for this in e2e/up

For the e2e/up test I think I'll need a zip package containing a dummy feature and stores it on a private package registry of the devpod repo. Would it be ok for you ? And are you able to store this package in a private registry?

Added a test and simulates the HTTP server during execution.

Great, thanks

@aacebedo aacebedo force-pushed the aacebedo/add_http_headers branch 2 times, most recently from 34e0b0b to e8ea210 Compare October 16, 2023 20:40
pascalbreuninger
pascalbreuninger approved these changes Oct 17, 2023
View reviewed changes
Copy link
Member

@pascalbreuninger pascalbreuninger left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM

edit: @aacebedo the test fails in CI

@aacebedo
Copy link
Contributor Author

aacebedo commented Oct 18, 2023 •
edited
Loading

I rebased the PR and ran this test locally it was working correctly.
By taking a look to the test logs it seems it cannot find the resulting image and that's strange because I didn't change anything here so I may have activated a side-effect.

Can you give it a look please? I do not have access to the GH actions infrastructure so it is hard to me to reproduce it.

@pascalbreuninger
Copy link
Member

@aacebedo finally worked out, thanks a lot for your contribution and patience!

@pascalbreuninger pascalbreuninger merged commit 74bb3c5 into loft-sh:main Oct 19, 2023
3 checks passed
@aacebedo
Copy link
Contributor Author

@aacebedo finally worked out, thanks a lot for your contribution and patience!

Thanks to you too to have taken the time to review and merge this. It is going to be very useful to me.

@aacebedo
Copy link
Contributor Author

@pascalbreuninger just a small question, when do you plan to tag a new version of devpod?

@pascalbreuninger
Copy link
Member

@aacebedo We're waiting for #734 to be merged for the v0.4.0 release but we can cut an alpha version right now

@pascalbreuninger
Copy link
Member

@aacebedo It's released in v0.4.0 now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pascalbreuninger pascalbreuninger pascalbreuninger approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@aacebedo @pascalbreuninger