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

feat(pci-block-storage): add availability zone step for 3AZ region #14406

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

feat(pci-block-storage): add availability zone step for 3AZ region #14406

wants to merge 2 commits into from

Conversation

SimonChaumet
Copy link
Contributor

@SimonChaumet SimonChaumet commented Dec 3, 2024 •
edited
Loading

Question Answer
Branch? develop
Bug fix? no
New feature? yes
Breaking change? no
Tickets Fix #TAPC-1703
License BSD 3-Clause
  • Try to keep pull requests small so they can be easily reviewed.
  • Commits are signed-off
  • Only FR translations have been updated
  • Branch is up-to-date with target branch
  • Lint has passed locally
  • Standalone app was ran and tested locally
  • Ticket reference is mentioned in linked commits (internal only)
  • Breaking change is mentioned in relevant commits

Description

  • implements new 1AZ chips
  • implements new 3AZ regions with availability zones
  • implements new volume catalog route for better performances

Related

@SimonChaumet SimonChaumet force-pushed the feat/block-storage-3AZ branch from 6817ef1 to c7dbadd Compare December 10, 2024 09:38
@SimonChaumet SimonChaumet force-pushed the feat/block-storage-3AZ branch from c7dbadd to 52cdc63 Compare December 20, 2024 10:44
@github-actions github-actions bot added the has conflicts Has conflicts to resolve before merging label Jan 7, 2025
@SimonChaumet SimonChaumet force-pushed the feat/block-storage-3AZ branch 4 times, most recently from 507aa7c to f6fc982 Compare January 9, 2025 17:07
@SimonChaumet SimonChaumet mentioned this pull request Jan 10, 2025
Merged
8 tasks
@SimonChaumet SimonChaumet force-pushed the feat/block-storage-3AZ branch from 90906a5 to 379fbf1 Compare January 13, 2025 08:00
@github-actions github-actions bot added dependencies Pull requests that update a dependency file and removed has conflicts Has conflicts to resolve before merging labels Jan 13, 2025
@github-actions GitHub Actions
Copy link
Contributor

yarn.lock changes

Click to toggle table visibility
Name Status Previous Current
@ovh-ux/manager-pci-common UPDATED 0.14.2 0.15.0

@SimonChaumet SimonChaumet force-pushed the feat/block-storage-3AZ branch from cf97ac5 to 209036b Compare January 16, 2025 13:04
@SimonChaumet
feat(pci-block-storage): add availability zone step
a022a6f 
ref: TAPC-2111

Signed-off-by: Simon Chaumet <[email protected]>
@SimonChaumet SimonChaumet force-pushed the feat/block-storage-3AZ branch from 209036b to f9b1b8e Compare January 16, 2025 14:15
@SimonChaumet SimonChaumet marked this pull request as ready for review January 16, 2025 15:36
@SimonChaumet SimonChaumet requested review from a team as code owners January 16, 2025 15:36
fredericvilcot
fredericvilcot previously approved these changes Jan 16, 2025
View reviewed changes
Copy link
Contributor

@fredericvilcot fredericvilcot left a comment

Choose a reason for hiding this comment

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

Great work, I wrote down some minor remarks, it's up to you 😌

packages/manager/apps/pci-block-storage/src/api/data/catalog.ts Show resolved Hide resolved
packages/manager/apps/pci-block-storage/src/api/data/catalog.ts
Comment on lines +67 to +73
.reduce<number | null>(
(leastPrice, modelLeastPrice) =>
leastPrice === null
? modelLeastPrice
: Math.min(modelLeastPrice, leastPrice),
null,
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like a duplicate of getLeastPrice, maybe we could refactor a bit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if this is really adapted to create a reduceMinPrice method

packages/manager/apps/pci-block-storage/src/pages/new/components/AvailabilityZoneStep.tsx Outdated Show resolved Hide resolved
packages/manager/apps/pci-block-storage/src/pages/new/components/AvailabilityZoneStep.tsx
}: Readonly<AvailabilityZoneStepProps>) {
const { t } = useTranslation('stepper');

const [selectedZone, setSelectedZone] = useState<string | undefined>(
Copy link
Contributor

Choose a reason for hiding this comment

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

If TilesInputComponent component accepts null as value, prefer instantiate it with null over undefined

packages/manager/apps/pci-block-storage/src/pages/new/components/LocationStep.component.tsx
Comment on lines +56 to +58
const [selectedLocalisation, setSelectedLocalisation] = useState<
TLocalisation
>(undefined);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const [selectedLocalisation, setSelectedLocalisation] = useState<
TLocalisation
>(undefined);
const [selectedLocalisation, setSelectedLocalisation] = useState<
TLocalisation | null
>(null);

packages/manager/apps/pci-block-storage/src/pages/new/form.type.ts
Comment on lines +1 to +2
import { TVolumeAddon, TVolumePricing } from '@/api/data/catalog';
import { TRegion } from '@/api/data/regions';
Copy link
Contributor

Choose a reason for hiding this comment

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

As a global improvement (in a future refactoring), we should not make the form depending on DTO but rather than front-end entities to prevent the µapp to be coupled to API

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this will be removed when we remove the stepper and use the new region selector

@SimonChaumet SimonChaumet force-pushed the feat/block-storage-3AZ branch from f9b1b8e to 08f6569 Compare January 16, 2025 16:33
@SimonChaumet SimonChaumet changed the title feat(pci-block-storage): add availability zone step feat(pci-block-storage): add availability zone step for 3AZ region Jan 17, 2025
@SimonChaumet SimonChaumet force-pushed the feat/block-storage-3AZ branch from 08f6569 to 348b94f Compare January 17, 2025 11:23
@SimonChaumet
feat(pci-block-storage): add deploy zone
f262e05 
ref: TAPC-2112

Signed-off-by: Simon Chaumet <[email protected]>
@SimonChaumet SimonChaumet force-pushed the feat/block-storage-3AZ branch from 348b94f to f262e05 Compare January 17, 2025 14:29
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

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

@fredericvilcot fredericvilcot fredericvilcot approved these changes

@aottr aottr Awaiting requested review from aottr aottr is a code owner automatically assigned from ovh/team-manager-dev-reactjs

@oalkabouss oalkabouss Awaiting requested review from oalkabouss oalkabouss is a code owner automatically assigned from ovh/team-manager-dev-reactjs

@kqesar kqesar Awaiting requested review from kqesar kqesar is a code owner automatically assigned from ovh/su-digital-tools-core-stack

@seven-amid seven-amid Awaiting requested review from seven-amid seven-amid is a code owner automatically assigned from ovh/su-digital-tools-core-stack

@darsene darsene Awaiting requested review from darsene darsene is a code owner automatically assigned from ovh/team-manager-dev-angularjs

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
dependencies Pull requests that update a dependency file feature New feature translation required
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@SimonChaumet @fredericvilcot