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

Icône d’un routeur physique anormalement large #42

Closed
Mazwak opened this issue Oct 8, 2021 · 11 comments
Closed

Icône d’un routeur physique anormalement large #42

Mazwak opened this issue Oct 8, 2021 · 11 comments
Assignees
Labels
bug Something isn't working

Comments

@Mazwak
Copy link

Mazwak commented Oct 8, 2021

Bonjour

Dans la vue de l’infrastrcture physique, l’icône d’un routeur prend une place anormalement large.

image

@Mazwak
Copy link
Author

Mazwak commented Oct 8, 2021

Testé avec Firefox 93 et Edge 94

@dbarzin dbarzin self-assigned this Oct 8, 2021
@dbarzin dbarzin added the bug Something isn't working label Oct 8, 2021
@dbarzin
Copy link
Owner

dbarzin commented Oct 8, 2021

As-tu bien la dernière version du projet ?
Pourrais-tu poster la source de la page ?

@Mazwak
Copy link
Author

Mazwak commented Oct 12, 2021

Il se trouve que Laravel, pour un « char » dans Postgre crée un « bpchar » au lieu d’un « varchar ».

« bpchar », c’est « blank padded character », donc c’est le nom du routeur qui fait 255 caractères.

Tous les autres « name » dans mercator sont des « string ». Et du coup, cela devient des « varchar ».

@Mazwak
Copy link
Author

Mazwak commented Oct 12, 2021

Je suppose que le nom d’un routeur physique devrait être aussi un « string ».
Je ne sais pas s’il est possible de faire "table->string("name")->change();" sur une colonne « char ».

Sinon, il faut le faire en SQL, mais ça fait autant de requête que de base de données supportées.

@dbarzin
Copy link
Owner

dbarzin commented Oct 12, 2021

Laravel devrait fonctionner avec PostgreSQL 9.6+ ( https://laravel.com/docs/8.x/database )
Quelle version de PostgreSQL utilises-tu ? Avec quel OS ?

@Mazwak
Copy link
Author

Mazwak commented Oct 12, 2021

Nous nous sommes mal compris.
Cela marche très bien avec Postgre.

Mais la création de la colonne « name » pour « physical_routeur » utilise $table->char() au lieu de $table->string().
Tous les autres « name » utilisent « string() ».

à comparer avec

Je pense que la différence vient de cette phrase, de la documentation MySQL :

When CHAR values are retrieved, trailing spaces are removed unless the PAD_CHAR_TO_FULL_LENGTH SQL mode is enabled.

https://dev.mysql.com/doc/refman/8.0/en/char.html

alors que Postgre renvoie la chaîne avec les espaces à droite.

Je ne peux pas vérifier en dehors du boulot, mais j’utilise Postgre fourni dans une Ubuntu 20.04. Probablement 12.x.

@dbarzin
Copy link
Owner

dbarzin commented Oct 12, 2021

Corrigé, le type du champ nom a été modifié de "char" en "string".
pour appliquer la modification :
git pull
php artisan migrate

@Mazwak
Copy link
Author

Mazwak commented Oct 13, 2021

Cela ne fonctionne pas avec Postgre.
J’ai regardé la migration, et elle est spécifique à MySQL.

J’ai essayé de faire fonctionner avec les migrations Laravel et ce post :
https://medium.com/@matriphe/adding-custom-char-type-in-laravel-migration-780d8a9cac29

La migration se déroule sans erreur, mais la colonne ne change pas de type.

Je ne pourrai pas regarder plus aujourd’hui.

Il y a sûrement moyen de la faire comme pour MySQL, mais cela fera une requête spécifique par base de données.

@dbarzin
Copy link
Owner

dbarzin commented Oct 13, 2021

Voici la commande SQL pour PostgresSQL:
ALTER TABLE physical_routers ALTER COLUMN name TYPE varchar(255);
Pour la migration, il faut supprimer le code de la fonction up() et exécuter.

@Mazwak
Copy link
Author

Mazwak commented Oct 14, 2021

Merci pour la requête.

Par contre, cela touchera tous ceux qui utilise postgre.

Personnellement, je pense que ce serait mieux de modifier la migration initiale.

Le nombre de personne utilisant mercator actuellement avec une base postgre doit être assez limité, et cela permettra d’éviter le problème totalement pour tous les nouveaux utilisateurs.

@dbarzin
Copy link
Owner

dbarzin commented Oct 14, 2021

Bonne remarque, c'est fait.
Merci !

@dbarzin dbarzin closed this as completed Oct 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants