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(ws): initial commit for backend #7

Merged
merged 3 commits into from
May 31, 2024

Conversation

ederign
Copy link
Member

@ederign ederign commented May 22, 2024

Scaffolding of the backend.

In this PR:

@thesuperzapper thesuperzapper changed the title feat(nb): Scaffolding of backend(/workspaces/backend) feat(ws): initial commit for backend May 23, 2024
workspaces/backend/.gitignore Outdated Show resolved Hide resolved
workspaces/backend/.golangci.yml Outdated Show resolved Hide resolved
workspaces/backend/README.md Outdated Show resolved Hide resolved
workspaces/backend/README.md Outdated Show resolved Hide resolved
workspaces/backend/api/middleware.go Outdated Show resolved Hide resolved
go 1.22.2

require (
github.com/julienschmidt/httprouter v1.3.0
Copy link
Member

Choose a reason for hiding this comment

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

Just checking on if this is a currently maintained package, as it last release was in 2019:

https://github.com/julienschmidt/httprouter/releases/tag/v1.3.0

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm open to exploring new routers if necessary, but I'm based on this flow chart here: https://www.alexedwards.net/blog/which-go-router-should-i-use

Copy link

@jiridanek jiridanek May 29, 2024

Choose a reason for hiding this comment

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

The package does not have any dependencies! I think it is not very dangerous to use it https://github.com/julienschmidt/httprouter/blob/master/go.mod

workspaces/backend/integrations/constants.go Outdated Show resolved Hide resolved
workspaces/backend/integrations/k8s.go Outdated Show resolved Hide resolved
@ederign
Copy link
Member Author

ederign commented May 29, 2024

@thesuperzapper @jiridanek I've updated my PR as we discussed.

I'll open a new issue to discuss the k8 client.

.gitignore Outdated

## backend
workspaces/backend/bin
Copy link

@jiridanek jiridanek May 29, 2024

Choose a reason for hiding this comment

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

maybe create a separate .gitignore file for this in workspaces/backend?

Copy link
Member Author

Choose a reason for hiding this comment

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

We had this, but @thesuperzapper asked to remove it on the last review.

Choose a reason for hiding this comment

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

I understand he asked to just remove the last few lines from that (because they would duplicate content of the top level .gitignore)

Comment on lines +7 to +9
.PHONY: help
help: ## Display this help.
@awk 'BEGIN {FS = ":.*##"; printf "\nUsage:\n make \033[36m<target>\033[0m\n"} /^[a-zA-Z_0-9-]+:.*?##/ { printf " \033[36m%-15s\033[0m %s\n", $$1, $$2 } /^##@/ { printf "\n\033[1m%s\033[0m\n", substr($$0, 5) } ' $(MAKEFILE_LIST)

Choose a reason for hiding this comment

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

Just fyi, make version 4.4.1 will natively support a new --print-targets option, which makes this solution unnecessary, if you can have new enough make

Signed-off-by: Eder Ignatowicz <[email protected]>
Signed-off-by: Eder Ignatowicz <[email protected]>
Copy link
Member

@thesuperzapper thesuperzapper left a comment

Choose a reason for hiding this comment

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

@ederign thanks, lets merge this and work from here!

Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jiridanek, thesuperzapper

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@thesuperzapper
Copy link
Member

/lgtm

@google-oss-prow google-oss-prow bot added the lgtm label May 31, 2024
@google-oss-prow google-oss-prow bot merged commit 9a945fb into kubeflow:notebooks-v2 May 31, 2024
2 checks passed
Adembc pushed a commit to Adembc/notebooks that referenced this pull request Jun 2, 2024
* feat(ws): initial commit for backend

Signed-off-by: Eder Ignatowicz <[email protected]>

* Fixing docker build

Signed-off-by: Eder Ignatowicz <[email protected]>

* Fixing git ignore

Signed-off-by: Eder Ignatowicz <[email protected]>

---------

Signed-off-by: Eder Ignatowicz <[email protected]>
Signed-off-by: Adem Baccara <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants