-
Notifications
You must be signed in to change notification settings - Fork 56
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
Conversation
fix: remove "Environment Variables" in expanded state (Remote Deploy)
export function DeploymentDetail({ dseq }: React.PropsWithChildren<{ dseq: string }>) { | ||
export function DeploymentDetail() { | ||
const dseq = (useParams()?.dseq as string) ?? ""; | ||
console.log(dseq); |
There was a problem hiding this comment.
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() { |
There was a problem hiding this comment.
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?
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); |
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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"); |
There was a problem hiding this comment.
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> }) => { |
There was a problem hiding this comment.
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()))); |
There was a problem hiding this comment.
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"; |
There was a problem hiding this comment.
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 }) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why remove that?
There was a problem hiding this comment.
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: `--- |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
Refactor: Implement structural changes and add TypeScript types for better code clarity and type safety
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; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (localDeploymentData && localDeploymentData.manifest) { | |
if (localDeploymentData?.manifest) { |
showOutsideDeploymentMessage: boolean; | ||
editedManifest: string; | ||
deploymentVersion: string | null; | ||
setDeploymentVersion: (value: React.SetStateAction<string | null>) => void; |
There was a problem hiding this comment.
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
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 }) => { |
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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 { |
There was a problem hiding this comment.
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"; |
There was a problem hiding this comment.
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) { |
There was a problem hiding this comment.
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<{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const tokens = atomWithStorage<{ | |
export const tokens = atomWithStorage<{ |
There was a problem hiding this comment.
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 => { |
There was a problem hiding this comment.
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"; |
There was a problem hiding this comment.
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.
fix : services for remote api and types
Pull changes
fix: fetch deploy.yml from awesome akash
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 |
This is a PR that addresses the new feature addition, enabling deployment directly from GitHub, GitLab, or Bitbucket onto the Akash Network.
Issue: #226