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

feature: Enable Deployment from Git Repositories to Akash Network #358

Closed
wants to merge 76 commits into from

Conversation

HoomanDgtl
Copy link

This is a PR that addresses the new feature addition, enabling deployment directly from GitHub, GitLab, or Bitbucket onto the Akash Network.

Issue: #226

export function DeploymentDetail({ dseq }: React.PropsWithChildren<{ dseq: string }>) {
export function DeploymentDetail() {
const dseq = (useParams()?.dseq as string) ?? "";
console.log(dseq);
Copy link
Collaborator

Choose a reason for hiding this comment

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

✂️

@@ -28,7 +31,10 @@ import { DeploymentSubHeader } from "./DeploymentSubHeader";
import { LeaseRow } from "./LeaseRow";
import { ManifestUpdate } from "./ManifestUpdate";

export function DeploymentDetail({ dseq }: React.PropsWithChildren<{ dseq: string }>) {
export function DeploymentDetail() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the reason of removing the props and using the useParams hook?

Comment on lines 139 to 143
const [remoteDeploy, setRemoteDeploy] = useState<boolean>(false);
const [repo, setRepo] = useState<string | null>(null);
const [editedManifest, setEditedManifest] = useState<string | null>(null);
const [deploymentVersion, setDeploymentVersion] = useState<string | null>(null);
const [showOutsideDeploymentMessage, setShowOutsideDeploymentMessage] = useState(false);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please move all these variables at the top of the components with over variables.

useEffect(() => {
const init = async () => {
const localDeploymentData = getDeploymentLocalData(deployment?.dseq || "");
console.log("localDeploymentData", !!localDeploymentData && !!localDeploymentData?.manifest);
Copy link
Collaborator

Choose a reason for hiding this comment

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

✂️


setDeploymentVersion(version);
} else {
console.log("klsj");
Copy link
Collaborator

Choose a reason for hiding this comment

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

✂️

};
export default Rollback;

const Field = ({ data, control }: { data: any; control: Control<SdlBuilderFormValuesType> }) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Extract this into a separate file.

value={value}
onChange={e => {
setValue(e.target.value);
// setFilteredData(data.filter((item: any) => item.name.toLowerCase().includes(e.target.value.toLowerCase())));
Copy link
Collaborator

Choose a reason for hiding this comment

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

✂️

@@ -0,0 +1,79 @@
import { Control } from "react-hook-form";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Move this under the /utils folder

}
};
}
// export async function getServerSideProps({ params }) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why remove that?

Choose a reason for hiding this comment

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

This was done to deploy the frontend on a third-party vendor. This was causing issues with build. This won't be removed, will revert it to previous state.

description: "Get started with a simple linux Ubuntu server!",
githubUrl: "",
valuesToChange: [],
content: `---
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we could put this template in the awesome akash repo, so when you need to update it we don't need to release a new version of the console.

Choose a reason for hiding this comment

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

Done, now we are fetching this from awesome akash repo.

Copy link
Collaborator

Choose a reason for hiding this comment

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

So we can remove it from here?

const { address, isWalletLoaded } = useWallet();
const { isSettingsInit } = useSettings();
const [leaseRefs, setLeaseRefs] = useState<Array<any>>([]);
const [deploymentManifest, setDeploymentManifest] = useState<string | null>(null);
const remoteDeploy: boolean = editedManifest && isRedeployImage(editedManifest) ? true : false;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const remoteDeploy: boolean = editedManifest && isRedeployImage(editedManifest) ? true : false;
const remoteDeploy: boolean = !!editedManifest && isRedeployImage(editedManifest);

const init = async () => {
const localDeploymentData = getDeploymentLocalData(deployment?.dseq || "");

if (localDeploymentData && localDeploymentData.manifest) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if (localDeploymentData && localDeploymentData.manifest) {
if (localDeploymentData?.manifest) {

showOutsideDeploymentMessage: boolean;
editedManifest: string;
deploymentVersion: string | null;
setDeploymentVersion: (value: React.SetStateAction<string | null>) => void;
Copy link
Collaborator

Choose a reason for hiding this comment

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

There's a type from react for such functions

Suggested change
setDeploymentVersion: (value: React.SetStateAction<string | null>) => void;
setDeploymentVersion: Dispatch<SetStateAction<string | null>>;

setIsEditingEnv: Dispatch<SetStateAction<boolean | number>>;
};

export const EnvVarList: React.FunctionComponent<Props> = ({ currentService, setIsEditingEnv, serviceIndex }) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

have you checked this https://github.com/akash-network/console/blob/main/apps/deploy-web/src/utils/sdl/transformCustomSdlFields.ts? We used this to mutate sdl eventually

);

useEffect(() => {
setOpen(true);
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not make it true by default then?

@@ -0,0 +1,12 @@
import { useEffect } from "react";

export function useWhenNot<T>(condition: T, run: () => void, deps: unknown[] = [], ifNot: () => void): void {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is inherently misleading. It's read as run this function if condition is false. The last argument just raises questions and you gotta check implementation.

generally you can use multiple calls to useWhen

const condition: boolean

useWhen(condition, cb)
useWhen(!condition, cb)

@@ -0,0 +1,171 @@
import { useMutation, useQuery } from "react-query";
Copy link
Collaborator

Choose a reason for hiding this comment

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

definitely

@@ -0,0 +1,30 @@
import axios from "axios";

export default function handler(req, res) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

what does this handle? can we get a descriptive names for this and other ones?


import { OAuth } from "@src/components/remote-deploy/utils";

const tokens = atomWithStorage<{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const tokens = atomWithStorage<{
export const tokens = atomWithStorage<{

Copy link
Collaborator

Choose a reason for hiding this comment

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

no need to export object below

if (code) {
axios
.post(tokenUrl, params.toString(), { headers })
.then(response => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not async/await?

@@ -0,0 +1,30 @@
import axios from "axios";
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd say business logic should be abstracted from the handlers into dedicated modules. Would be better if those are classes.

@baktun14
Copy link
Collaborator

Can you add the env vars you added to the config files? https://github.com/akash-network/console/blob/main/apps/deploy-web/src/config/browser-env.config.ts

@HoomanDgtl HoomanDgtl closed this Sep 21, 2024
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.

5 participants